* [PATCH] devfs v181 available @ 2001-06-18 6:01 Richard Gooch 2001-06-18 6:33 ` Alexander Viro 2001-06-18 6:40 ` Richard Gooch 0 siblings, 2 replies; 7+ messages in thread From: Richard Gooch @ 2001-06-18 6:01 UTC (permalink / raw) To: linux-kernel, devfs-announce-list Hi, all. Version 181 of my devfs patch is now available from: http://www.atnf.csiro.au/~rgooch/linux/kernel-patches.html The devfs FAQ is also available here. Patch directly available from: ftp://ftp.??.kernel.org/pub/linux/kernel/people/rgooch/v2.4/devfs-patch-current.gz AND: ftp://ftp.atnf.csiro.au/pub/people/rgooch/linux/kernel-patches/v2.4/devfs-patch-current.gz This is against 2.4.6-pre3. Highlights of this release: - Answered question posed by Al Viro and removed his comments from <devfs_open> - Moved setting of registered flag after other fields are changed - Fixed race between <devfsd_close> and <devfsd_notify_one> - Global VFS changes added bogus BKL to devfsd_close(): removed - Widened locking in <devfs_readlink> and <devfs_follow_link> - Replaced <devfsd_read> stack usage with <devfsd_ioctl> kmalloc - Simplified locking in <devfsd_ioctl> and fixed memory leak Regards, Richard.... Permanent: rgooch@atnf.csiro.au Current: rgooch@ras.ucalgary.ca ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] devfs v181 available 2001-06-18 6:01 [PATCH] devfs v181 available Richard Gooch @ 2001-06-18 6:33 ` Alexander Viro 2001-06-18 6:40 ` Richard Gooch 1 sibling, 0 replies; 7+ messages in thread From: Alexander Viro @ 2001-06-18 6:33 UTC (permalink / raw) To: Richard Gooch; +Cc: linux-kernel, devfs-announce-list On Mon, 18 Jun 2001, Richard Gooch wrote: > - Widened locking in <devfs_readlink> and <devfs_follow_link> No, you hadn't. Both vfs_readlink() and vfs_follow_link() are blocking functions, so BKL is worthless there. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] devfs v181 available 2001-06-18 6:01 [PATCH] devfs v181 available Richard Gooch 2001-06-18 6:33 ` Alexander Viro @ 2001-06-18 6:40 ` Richard Gooch 2001-06-18 7:09 ` Alexander Viro 2001-06-18 15:15 ` Richard Gooch 1 sibling, 2 replies; 7+ messages in thread From: Richard Gooch @ 2001-06-18 6:40 UTC (permalink / raw) To: Alexander Viro; +Cc: linux-kernel, devfs-announce-list Alexander Viro writes: > > > On Mon, 18 Jun 2001, Richard Gooch wrote: > > > - Widened locking in <devfs_readlink> and <devfs_follow_link> > > No, you hadn't. Both vfs_readlink() and vfs_follow_link() are blocking > functions, so BKL is worthless there. Huh? The BKL will protect against other operations which might cause the devfs entry to be unregistered, where those other operations also grab the BKL. So, it's an improvement. Sure, some operations may cause unregistration without grabbing the BKL, but that's orthogonal (and requires more extensive changes). If this "widening" is of no use, then what use are the existing grabs of the BKL in those functions? You're the one who added them in the first place. Regards, Richard.... Permanent: rgooch@atnf.csiro.au Current: rgooch@ras.ucalgary.ca ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] devfs v181 available 2001-06-18 6:40 ` Richard Gooch @ 2001-06-18 7:09 ` Alexander Viro 2001-06-18 15:15 ` Richard Gooch 1 sibling, 0 replies; 7+ messages in thread From: Alexander Viro @ 2001-06-18 7:09 UTC (permalink / raw) To: Richard Gooch; +Cc: linux-kernel, devfs-announce-list On Mon, 18 Jun 2001, Richard Gooch wrote: > Alexander Viro writes: > > > > > > On Mon, 18 Jun 2001, Richard Gooch wrote: > > > > > - Widened locking in <devfs_readlink> and <devfs_follow_link> > > > > No, you hadn't. Both vfs_readlink() and vfs_follow_link() are blocking > > functions, so BKL is worthless there. > > Huh? The BKL will protect against other operations which might cause > the devfs entry to be unregistered, where those other operations also > grab the BKL. So, it's an improvement. BKL is released as soon as you block. You _do_ regain it when you get the next timeslice, but in the meanwhile anything could happen. > Sure, some operations may cause unregistration without grabbing the Irrelevant. BKL provides an exclusion only on non-blocking areas. > BKL, but that's orthogonal (and requires more extensive changes). If > this "widening" is of no use, then what use are the existing grabs of > the BKL in those functions? You're the one who added them in the first > place. _Moved_ them there from the callers of these functions. And AFAICS you do need BKL for get_devfs_entry_...(); otherwise relocation of the table will be able to screw you inside of that function. Now, it will merrily screw you anyway in a lot of places, but that's another story. BTW, free advice: when you are checking some condition treat the result as something that can expire. And don't rely on it past the moment when it might expired. E.g. in case of de->registered result expires as soon as you do unlock_kernel() _or_ do anything that might block. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] devfs v181 available 2001-06-18 6:40 ` Richard Gooch 2001-06-18 7:09 ` Alexander Viro @ 2001-06-18 15:15 ` Richard Gooch 2001-06-18 16:55 ` Alexander Viro 2001-06-18 18:53 ` Richard Gooch 1 sibling, 2 replies; 7+ messages in thread From: Richard Gooch @ 2001-06-18 15:15 UTC (permalink / raw) To: Alexander Viro; +Cc: linux-kernel, devfs-announce-list Alexander Viro writes: > On Mon, 18 Jun 2001, Richard Gooch wrote: > > Alexander Viro writes: > > > On Mon, 18 Jun 2001, Richard Gooch wrote: > > > > - Widened locking in <devfs_readlink> and <devfs_follow_link> > > > > > > No, you hadn't. Both vfs_readlink() and vfs_follow_link() are blocking > > > functions, so BKL is worthless there. > > > > Huh? The BKL will protect against other operations which might cause > > the devfs entry to be unregistered, where those other operations also > > grab the BKL. So, it's an improvement. > > BKL is released as soon as you block. You _do_ regain it when you get > the next timeslice, but in the meanwhile anything could happen. > > > Sure, some operations may cause unregistration without grabbing the > > Irrelevant. BKL provides an exclusion only on non-blocking areas. Yeah, I know all that. > > BKL, but that's orthogonal (and requires more extensive changes). If > > this "widening" is of no use, then what use are the existing grabs of > > the BKL in those functions? You're the one who added them in the first > > place. > > _Moved_ them there from the callers of these functions. And AFAICS > you do need BKL for get_devfs_entry_...(); otherwise relocation of > the table will be able to screw you inside of that function. Now, it > will merrily screw you anyway in a lot of places, but that's another > story. OK, so it was another global change. > BTW, free advice: when you are checking some condition treat the > result as something that can expire. And don't rely on it past the > moment when it might expired. E.g. in case of de->registered result > expires as soon as you do unlock_kernel() _or_ do anything that > might block. Yeah, I know. Fear not: I'll be adding proper locks (spinlocks and semaphores) to the code, continuing the work I started this weekend. I don't like the BKL anyway (too coarse grained), and hope to see it removed entirely one day. Question: assuming data fed to vfs_follow_link() is "safe", does it need the BKL? I can see that vfs_readlink() obviously doesn't need it. From reading Documentation/filesystems/Locking I suspect it doesn't need the BKL, but the way I read it says "follow_link() method does not *have* the BKL already". But that doesn't explicitely say whether vfs_follow_link() needs it. Regards, Richard.... Permanent: rgooch@atnf.csiro.au Current: rgooch@ras.ucalgary.ca ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] devfs v181 available 2001-06-18 15:15 ` Richard Gooch @ 2001-06-18 16:55 ` Alexander Viro 2001-06-18 18:53 ` Richard Gooch 1 sibling, 0 replies; 7+ messages in thread From: Alexander Viro @ 2001-06-18 16:55 UTC (permalink / raw) To: Richard Gooch; +Cc: linux-kernel, devfs-announce-list On Mon, 18 Jun 2001, Richard Gooch wrote: > > Irrelevant. BKL provides an exclusion only on non-blocking areas. > > Yeah, I know all that. So what the hell are you talking about? > > _Moved_ them there from the callers of these functions. And AFAICS > > you do need BKL for get_devfs_entry_...(); otherwise relocation of > > the table will be able to screw you inside of that function. Now, it > > will merrily screw you anyway in a lot of places, but that's another > > story. > > OK, so it was another global change. Moving BKL into the ->readlink() and ->follow_link()? Sure, it was a global change. About a year ago. > Question: assuming data fed to vfs_follow_link() is "safe", does it ^^^^^^^^ > need the BKL? I can see that vfs_readlink() obviously doesn't need > it. From reading Documentation/filesystems/Locking I suspect it > doesn't need the BKL, but the way I read it says "follow_link() method > does not *have* the BKL already". But that doesn't explicitely say > whether vfs_follow_link() needs it. vfs_follow_link() doesn't need it. Moreover, if data fed to it is unsafe without BKL, you are screwed even if you take BKL. So assumption above is bogus - you _never_ need BKL on that call. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] devfs v181 available 2001-06-18 15:15 ` Richard Gooch 2001-06-18 16:55 ` Alexander Viro @ 2001-06-18 18:53 ` Richard Gooch 1 sibling, 0 replies; 7+ messages in thread From: Richard Gooch @ 2001-06-18 18:53 UTC (permalink / raw) To: Alexander Viro; +Cc: linux-kernel, devfs-announce-list Alexander Viro writes: > > > On Mon, 18 Jun 2001, Richard Gooch wrote: > > > > Irrelevant. BKL provides an exclusion only on non-blocking areas. > > > > Yeah, I know all that. > > So what the hell are you talking about? Never mind. We seem to be talking at cross purposes. We both know how the BKL works and the implications, so there's not much point beating our heads trying to communicate redundant information :-) > > > _Moved_ them there from the callers of these functions. And AFAICS > > > you do need BKL for get_devfs_entry_...(); otherwise relocation of > > > the table will be able to screw you inside of that function. Now, it > > > will merrily screw you anyway in a lot of places, but that's another > > > story. > > > > OK, so it was another global change. > > Moving BKL into the ->readlink() and ->follow_link()? Sure, it was a global > change. About a year ago. > > > Question: assuming data fed to vfs_follow_link() is "safe", does it > ^^^^^^^^ > > need the BKL? I can see that vfs_readlink() obviously doesn't need > > it. From reading Documentation/filesystems/Locking I suspect it > > doesn't need the BKL, but the way I read it says "follow_link() method > > does not *have* the BKL already". But that doesn't explicitely say > > whether vfs_follow_link() needs it. > > vfs_follow_link() doesn't need it. Moreover, if data fed to it is > unsafe without BKL, you are screwed even if you take BKL. So > assumption above is bogus - you _never_ need BKL on that call. OK, you didn't see what I was driving at. If I had said "if my data is protected by a semaphore, do I still need the BKL for vfs_follow_link()" I guess it would have been clearer. Anyway, you've answered my question, thanks. Regards, Richard.... Permanent: rgooch@atnf.csiro.au Current: rgooch@ras.ucalgary.ca ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2001-06-18 18:53 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2001-06-18 6:01 [PATCH] devfs v181 available Richard Gooch 2001-06-18 6:33 ` Alexander Viro 2001-06-18 6:40 ` Richard Gooch 2001-06-18 7:09 ` Alexander Viro 2001-06-18 15:15 ` Richard Gooch 2001-06-18 16:55 ` Alexander Viro 2001-06-18 18:53 ` Richard Gooch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).