[PATCH] devfs v212 available

[PATCH] devfs v212 available

[Richard Gooch]

  Hi, all. Version 212 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.5/devfs-patch-current.gz

AND:
ftp://ftp.atnf.csiro.au/pub/people/rgooch/linux/kernel-patches/v2.5/devfs-patch-current.gz

NOTE: kernel 2.5.1 and later require devfsd-v1.3.19 or later.

This is against 2.5.14. Highlights of this release:

- Added BKL to <devfs_open> because drivers still need it

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] devfs v212 available

[Alexander Viro]

On Sat, 11 May 2002, Richard Gooch wrote:

> This is against 2.5.14. Highlights of this release:
> 
> - Added BKL to <devfs_open> because drivers still need it

Sigh...  Look at the callers of check_disc_changed() and check what's
going on with traversing directory contents there.

Re: [PATCH] devfs v212 available

[Richard Gooch]

Alexander Viro writes:
> On Sat, 11 May 2002, Richard Gooch wrote:
> > This is against 2.5.14. Highlights of this release:
> > 
> > - Added BKL to <devfs_open> because drivers still need it
> 
> Sigh...  Look at the callers of check_disc_changed() and check
> what's going on with traversing directory contents there.

OK, I've had a look. There is indeed a race there. While it is safe
against module unloading, it isn't safe against removal of entries
from the directory. I'm considering some different options to fix this
(one is simple and obvious, the other will be a little more
efficient).

Question: can invalidate_device() and the bdops methods
check_media_change() and revalidate() be called with a lock held?

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] devfs v212 available

[Alexander Viro]

On Sun, 12 May 2002, Richard Gooch wrote:

> OK, I've had a look. There is indeed a race there. While it is safe
> against module unloading, it isn't safe against removal of entries
> from the directory. I'm considering some different options to fix this
> (one is simple and obvious, the other will be a little more
> efficient).
> 
> Question: can invalidate_device() and the bdops methods
> check_media_change() and revalidate() be called with a lock held?

Erm...  Depends on the nature of lock.  Spinlocks are out of question,
obviously (at the very least we reread partition table if disk had been
changed and you can't make that nonblocking ;-).  Semaphore might work,
but I would be very careful with deadlocks - the same rereading partition
table could add or remove devfs entries.

Could you describe what are you trying to achieve in the callers of
check_disc_changed()?  I'd been unable to deduce that from code and
some comments would be very welcome.

Re: [PATCH] devfs v212 available

[Richard Gooch]

Alexander Viro writes:
> 
> 
> On Sun, 12 May 2002, Richard Gooch wrote:
> 
> > OK, I've had a look. There is indeed a race there. While it is safe
> > against module unloading, it isn't safe against removal of entries
> > from the directory. I'm considering some different options to fix this
> > (one is simple and obvious, the other will be a little more
> > efficient).
> > 
> > Question: can invalidate_device() and the bdops methods
> > check_media_change() and revalidate() be called with a lock held?
> 
> Erm...  Depends on the nature of lock.  Spinlocks are out of
> question, obviously (at the very least we reread partition table if
> disk had been changed and you can't make that nonblocking ;-).
> Semaphore might work, but I would be very careful with deadlocks -
> the same rereading partition table could add or remove devfs
> entries.

Yeah, I think I'll go for the simpler and safer solution.

> Could you describe what are you trying to achieve in the callers of
> check_disc_changed()?  I'd been unable to deduce that from code and
> some comments would be very welcome.

There are two cases where we want to do a partition re-read:
- in lookup(), if the entry was not found
- in readdir().

The point of this is to make sure that the partition list is updated
(if media has changed) whenever user-space might care. Without the
above code, if the media changes, and thus the partitioning changes,
the list of partitions that devfs has is stale. Two things (at least)
could go wrong in this case:
- ls on directory reports partitions that are no longer valid
- a new partition cannot be accessed because the entry hasn't been
  registered yet.

The code prevents these problems.

Clear?

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca