* Re: loop device auto release patch [not found] <4E1F3BE0.8040506@ayan.net> @ 2011-07-15 19:12 ` Phillip Susi 2011-07-15 19:19 ` Andrew Morton 0 siblings, 1 reply; 6+ messages in thread From: Phillip Susi @ 2011-07-15 19:12 UTC (permalink / raw) To: Ayan George, Andrew Morton, linux-kernel [-- Attachment #1: Type: text/plain, Size: 319 bytes --] On 7/14/2011 2:56 PM, Ayan George wrote: > > Hi Philip, > > This is the patch I'd like to submit for the loop device. I'm in the > process of testing it now. I'm pretty confident it will work. Looks good to me. Forwarding to Andrew Morton. Andrew, please disregard my previous patch as I think this one is better. [-- Attachment #2: 0001-Always-invalidate-cleared-loop-block-devices.patch --] [-- Type: text/x-patch, Size: 2920 bytes --] >From c783ba6a26eff42d3cb0061e4fcb8ee8a16b3e67 Mon Sep 17 00:00:00 2001 From: Ayan George <ayan.george@canonical.com> Date: Thu, 14 Jul 2011 14:16:58 -0400 Subject: [PATCH] Always invalidate cleared loop block devices The API for loop_clr_fd() is confusing -- the second argument (bdev) isn't necessesary as struct loop_device contains a pointer to the block device it is assocated with. There is a cases where loop_clr_fd() is called with NULL for bdev which prevents the block device from ever being invalidated with invalidate_bdev() and prevents a uevent from being emitted. This patch removes the bdev argument from loop_clr_fd(), unconditionally invalidates lo->lo_device when cleared, and unconditionally emits a uevent for removed loops. Launchpad bug for reference: BugLink: https://bugs.launchpad.net/bugs/548546 Signed-off-by: Ayan George <ayan.george@canonical.com> --- drivers/block/loop.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 76c8da7..a40e12b 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -987,11 +987,13 @@ loop_init_xfer(struct loop_device *lo, struct loop_func_table *xfer, return err; } -static int loop_clr_fd(struct loop_device *lo, struct block_device *bdev) +static int loop_clr_fd(struct loop_device *lo) { struct file *filp = lo->lo_backing_file; gfp_t gfp = lo->old_gfp_mask; + struct block_device *bdev = lo->lo_device; + if (lo->lo_state != Lo_bound) return -ENXIO; @@ -1022,15 +1024,17 @@ static int loop_clr_fd(struct loop_device *lo, struct block_device *bdev) memset(lo->lo_encrypt_key, 0, LO_KEY_SIZE); memset(lo->lo_crypt_name, 0, LO_NAME_SIZE); memset(lo->lo_file_name, 0, LO_NAME_SIZE); - if (bdev) - invalidate_bdev(bdev); + + invalidate_bdev(bdev); + set_capacity(lo->lo_disk, 0); loop_sysfs_exit(lo); - if (bdev) { - bd_set_size(bdev, 0); - /* let user-space know about this change */ - kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE); - } + + bd_set_size(bdev, 0); + + /* let user-space know about this change */ + kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE); + mapping_set_gfp_mask(filp->f_mapping, gfp); lo->lo_state = Lo_unbound; /* This is safe: open() is still holding a reference. */ @@ -1298,7 +1302,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode, break; case LOOP_CLR_FD: /* loop_clr_fd would have unlocked lo_ctl_mutex on success */ - err = loop_clr_fd(lo, bdev); + err = loop_clr_fd(lo); if (!err) goto out_unlocked; break; @@ -1509,7 +1513,7 @@ static int lo_release(struct gendisk *disk, fmode_t mode) * In autoclear mode, stop the loop thread * and remove configuration after last close. */ - err = loop_clr_fd(lo, NULL); + err = loop_clr_fd(lo); if (!err) goto out_unlocked; } else { -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: loop device auto release patch 2011-07-15 19:12 ` loop device auto release patch Phillip Susi @ 2011-07-15 19:19 ` Andrew Morton 2011-07-15 22:28 ` Ayan George 2011-08-04 1:58 ` Phillip Susi 0 siblings, 2 replies; 6+ messages in thread From: Andrew Morton @ 2011-07-15 19:19 UTC (permalink / raw) To: Phillip Susi; +Cc: Ayan George, linux-kernel On Fri, 15 Jul 2011 15:12:50 -0400 Phillip Susi <psusi@cfl.rr.com> wrote: > On 7/14/2011 2:56 PM, Ayan George wrote: > > > > Hi Philip, > > > > This is the patch I'd like to submit for the loop device. I'm in the > > process of testing it now. I'm pretty confident it will work. > > Looks good to me. Forwarding to Andrew Morton. Andrew, please > disregard my previous patch as I think this one is better. Would prefer to wait until Ayan has completed testing it. It would be good if you were to test it too please. > > [0001-Always-invalidate-cleared-loop-block-devices.patch text/x-patch (2.9KB)] > >From c783ba6a26eff42d3cb0061e4fcb8ee8a16b3e67 Mon Sep 17 00:00:00 2001 > From: Ayan George <ayan.george@canonical.com> > Date: Thu, 14 Jul 2011 14:16:58 -0400 > Subject: [PATCH] Always invalidate cleared loop block devices > > The API for loop_clr_fd() is confusing -- the second argument (bdev) > isn't necessesary as struct loop_device contains a pointer to the block > device it is assocated with. > > There is a cases where loop_clr_fd() is called with NULL for bdev > which prevents the block device from ever being invalidated with > invalidate_bdev() and prevents a uevent from being emitted. > > This patch removes the bdev argument from loop_clr_fd(), unconditionally > invalidates lo->lo_device when cleared, and unconditionally emits a > uevent for removed loops. The patch appears to do two unrelated things. That's generally frowned upon, but doesn't bother me much if the patch is small. Still, splitting it into two patches (in which the bugfix is staged first) would be advantageous for people who might wish to backport the fix into earlier kernels. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: loop device auto release patch 2011-07-15 19:19 ` Andrew Morton @ 2011-07-15 22:28 ` Ayan George 2011-08-04 1:58 ` Phillip Susi 1 sibling, 0 replies; 6+ messages in thread From: Ayan George @ 2011-07-15 22:28 UTC (permalink / raw) To: Andrew Morton; +Cc: Phillip Susi, linux-kernel On 07/15/2011 03:19 PM, Andrew Morton wrote: > Still, splitting it into two patches (in which the bugfix is staged > first) would be advantageous for people who might wish to backport the > fix into earlier kernels. > Okay -- I'll submit two smaller patches and I'll early next week. -ayan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: loop device auto release patch 2011-07-15 19:19 ` Andrew Morton 2011-07-15 22:28 ` Ayan George @ 2011-08-04 1:58 ` Phillip Susi 2011-08-04 2:06 ` Andrew Morton 2011-08-04 2:13 ` Ayan George 1 sibling, 2 replies; 6+ messages in thread From: Phillip Susi @ 2011-08-04 1:58 UTC (permalink / raw) To: Andrew Morton; +Cc: Ayan George, linux-kernel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 07/15/2011 03:19 PM, Andrew Morton wrote: > The patch appears to do two unrelated things. That's generally frowned > upon, but doesn't bother me much if the patch is small. > > Still, splitting it into two patches (in which the bugfix is staged > first) would be advantageous for people who might wish to backport the > fix into earlier kernels. It looks like this got stalled and someone emailed me asking what happened to it. I'm not sure that splitting the patch in two makes sense. I don't see how it does two unrelated things. The uevent problem was caused by the argument being NULL. This patch just removes the argument since it is entirely unnecessary. Given that the argument is gone and can no longer be passed as NULL, the tests for NULL are rendered moot, and so removing them seems quite related. I suppose you could do the first and not the second, but then you would be leaving cruft behind. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk45/MAACgkQJ4UciIs+XuL7+gCfUA8TpE2SRRCo+/VzRJMCrwg6 OEsAmwWphcaIF/wdguyHch6gEizv1Tjb =pGGG -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: loop device auto release patch 2011-08-04 1:58 ` Phillip Susi @ 2011-08-04 2:06 ` Andrew Morton 2011-08-04 2:13 ` Ayan George 1 sibling, 0 replies; 6+ messages in thread From: Andrew Morton @ 2011-08-04 2:06 UTC (permalink / raw) To: Phillip Susi; +Cc: Ayan George, linux-kernel On Wed, 03 Aug 2011 21:58:24 -0400 Phillip Susi <psusi@cfl.rr.com> wrote: > On 07/15/2011 03:19 PM, Andrew Morton wrote: > > The patch appears to do two unrelated things. That's generally frowned > > upon, but doesn't bother me much if the patch is small. > > > > Still, splitting it into two patches (in which the bugfix is staged > > first) would be advantageous for people who might wish to backport the > > fix into earlier kernels. > > It looks like this got stalled and someone emailed me asking what > happened to it. I'm not sure that splitting the patch in two makes > sense. I don't see how it does two unrelated things. The uevent > problem was caused by the argument being NULL. This patch just removes > the argument since it is entirely unnecessary. Given that the argument > is gone and can no longer be passed as NULL, the tests for NULL are > rendered moot, and so removing them seems quite related. I suppose you > could do the first and not the second, but then you would be leaving > cruft behind. Am still waiting for someone to say that this is tested. And what is "my previous patch"? "emit uevent on auto release"? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: loop device auto release patch 2011-08-04 1:58 ` Phillip Susi 2011-08-04 2:06 ` Andrew Morton @ 2011-08-04 2:13 ` Ayan George 1 sibling, 0 replies; 6+ messages in thread From: Ayan George @ 2011-08-04 2:13 UTC (permalink / raw) To: psusi, akpm; +Cc: linux-kernel Phillip Susi <psusi@cfl.rr.com> wrote: > On 07/15/2011 03:19 PM, Andrew Morton wrote: > > The patch appears to do two unrelated things. That's generally frowned > > upon, but doesn't bother me much if the patch is small. > > > > Still, splitting it into two patches (in which the bugfix is staged > > first) would be advantageous for people who might wish to backport the > > fix into earlier kernels. > > It looks like this got stalled and someone emailed me asking what > happened to it. I'm not sure that splitting the patch in two makes > sense. I don't see how it does two unrelated things. I agree -- I can't think of a meaningful way to split it up. I am stalled in testing it however. I watched the values of lo->lo_device and bdev using systemtap and noticed cases where they could be out of sync which *might* make this patch invalid. I'd like a little more time to investigate. -ayan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-04 2:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4E1F3BE0.8040506@ayan.net>
2011-07-15 19:12 ` loop device auto release patch Phillip Susi
2011-07-15 19:19 ` Andrew Morton
2011-07-15 22:28 ` Ayan George
2011-08-04 1:58 ` Phillip Susi
2011-08-04 2:06 ` Andrew Morton
2011-08-04 2:13 ` Ayan George
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.