Messages in this thread | | | Date | Sat, 21 Jun 2003 20:31:24 +0100 | From | viro@parcelfa ... | Subject | Re: [RFC][PATCH] nbd driver for 2.5.72 |
| |
On Sat, Jun 21, 2003 at 10:39:12AM -0600, Lou Langholtz wrote: > >Why not put these into nbd_device? > > > I'd considered that and I'm reconsidering it again now. Not convinced > which way to go... Putting something as large as struct request_queue > within the nbd_device seems unbalanced somehow. Then again, until 2.5 > the request_queue was typically shared by multiple devices of the same > MAJOR so part of the way the code is has to do with the legacy code. > Like the nbd_lock spinlock array and the struct request_queue queue_lock > field. Along the lines you're pushing for, why not have struct > requests_queue's queue_lock field then be the spinlock itself instead of > just being a pointer to a spinlock???
Because often that lock protects driver-internal objects that are used by all queues.
Prefered variant (actually, we'll have to do it in 2.5 anyway) is to allocate request_queue dynamically. Just put a pointer to it into nbd_device.
BTW, could you please kill the ..._t silliness? There is nothing wroung with using 'struct nbd_device' directly.
> >>+static uint32_t request_magic; > >> > >> > > > >??? htonl(NBD_REQUEST_MAGIC) is perfectly OK in the place where you > >use it and more likely than not will give better code. > > > > > > > >>+static uint32_t reply_magic; > >> > >> > > > >Ditto. > > > What's wrong with having an explicit cache of this value that we can > rest assured doesn't in the worst case get compiled into multiple calls > to the htonl code?? Possible waste of one 4 byte memory location in the > worst compiler case or is there another problem?
htonl() honours constants. If it doesn't, we are in for much more serious problems, simply because a lot of codepaths in networking are using it. A lot. IOW, you are obfuscating code for no good reason (and add an extra memory access, thus giving actually worse code - it's not an optimisation at all). - 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/
| |