lkml.org 
[lkml]   [2010]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 2/6] block: push down BKL into .open and .release
    Date
    On Sunday 04 July 2010 10:01:46 Sam Ravnborg wrote:
    > > --- a/drivers/block/amiflop.c
    > > +++ b/drivers/block/amiflop.c
    > > @@ -1555,10 +1555,13 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
    > > int old_dev;
    > > unsigned long flags;
    > >
    > > + lock_kernel();
    > > old_dev = fd_device[drive];
    > >
    > > - if (fd_ref[drive] && old_dev != system)
    > > + if (fd_ref[drive] && old_dev != system) {
    > > + unlock_kernel();
    > > return -EBUSY;
    > > + }
    > >
    > > if (mode & (FMODE_READ|FMODE_WRITE)) {
    > > check_disk_change(bdev);
    > > @@ -1571,8 +1574,10 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
    > > fd_deselect (drive);
    > > rel_fdc();
    > >
    > > - if (wrprot)
    > > + if (wrprot) {
    > > + unlock_kernel();
    > > return -EROFS;
    > > + }
    > > }
    > > }
    > >
    > > @@ -1589,6 +1594,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
    > > printk(KERN_INFO "fd%d: accessing %s-disk with %s-layout\n",drive,
    > > unit[drive].type->name, data_types[system].name);
    > >
    > > + unlock_kernel();
    > > return 0;
    > > }
    >
    > Using goto for errorhandling here would have been nicer.

    I tried to minimize the chance for breaking stuff in code I cannot easily
    test build. As shown by the bug you found in my pktcdvd patch, changing the
    control flow of a function has a higher potential of introducing bugs,
    so I didn't do it for drivers that people don't care much about any more.

    > Also did you forget to include smp_locks.h?

    No, that was already there from the first patch.

    > Lot's of other places could benefit from improved goto error handling.
    > The driver maintainers should do this (if there is a maintainer).

    If that makes Jens accept my patches, I'd gladly change them this way.
    I agree that it's cleaner and I always write my own code like that,
    but when modifying (mostly) unmaintained code, my preference is on
    trying to ensure the patch is correct.


    > I did not find anything else going through this patch.

    Ok, thanks a lot for looking through this!

    Arnd


    \
     
     \ /
      Last update: 2010-07-04 23:49    [W:0.027 / U:29.100 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site