Re: [RFC][PATCH] nbd driver for 2.5+: fix locking issues with ioctl

Paul Clements (kernel@steeleye.com)
Wed, 25 Jun 2003 16:00:02 -0400 (EDT)


On Wed, 25 Jun 2003, Lou Langholtz wrote:

> Did Al's statements regarding this make this feel better? The code could
> be redone so that the queuelist is added to before the down but then
> more often it will have to be removed from since the lo->sock check

It wasn't that I was worried about someone calling down() under
spinlock, obviously that'd be a problem. However, if the queue_lock were
to be changed back to a semaphore (as it used to be, pre-2.4.16 or so)
this could lead to problems. I guess I was just wondering why the
tx_lock was pulled out of nbd_send_req(). It just seems to make the
code harder to follow with the overlapping locking, the duplicated
checks for sock == NULL, and it also means the tx_lock gets held
(a little bit) longer...

> that since the user space tool opened the socket to begin with, it seems
> a better design to have the user space tool do the close as well.

I agree, but I thought that the shutdown was causing different behavior...

> When
> nbd-client exits, it'd effect close of this socket anyway even if
> killed.

Does the socket close at exit have the same effect as a socket shutdown?
If so, then I guess the shutdown is unnecessary...

> So you'd prefer to have a new ioctl then to do this rolled together
> NBD_DO_IT function? Say NBD_RUN or something that takes the sock

I do like the combined ioctl, as it seems to make the code a little bit
cleaner and safer. But, it would also be nice to maintain compatibility
with the existing userland tools. Maybe if the driver could support both
new and old interfaces (at least for now), then users could gradually
move over to the new interface(s)?

--
Paul

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/