t; superblock with bd_fsfreeze_count always positive.
>
> Fixes: fcccf502540e3 ("filesystem freeze: implement generic freeze feature")
> Signed-off-by: Luis R. Rodriguez <mcg...@kernel.org>
Looks good to me. You can add:
Reviewed-by: Jan Kara <j...@suse.cz>
ct super_block *sb)
> */
> int thaw_super(struct super_block *sb)
> {
> + int error;
> +
> down_write(>s_umount);
> - return thaw_super_locked(sb);
> + error = thaw_super_locked(sb);
> + if (error) {
> + up_write(>s_umount);
> + goto out;
> + }
> +
> + deactivate_locked_super(sb);
> +
> +out:
> + return error;
> }
> EXPORT_SYMBOL(thaw_super);
> --
> 2.16.3
>
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
r...@i-love.sakura.ne.jp>
> Reported-by: syzbot <syzbot+c0cf869505e03bdf1...@syzkaller.appspotmail.com>
> Fixes: 5318ce7d46866e1d ("bdi: Shutdown writeback on all cgwbs in
> cgwb_bdi_destroy()")
> Cc: Tejun Heo <t...@kernel.org>
> Cc: Jan Kara <
5318ce7d46866e1d ("bdi:
> Shutdown writeback on all cgwbs in cgwb_bdi_destroy()"). It uses clear_bit()
> to clear WB_shutting_down bit so that threads waiting at wait_on_bit() will
> wake up. But clear_bit() itself does not wake up threads, does it? Who wakes
> them up (e.g. by calling wake_up_bit()) after clear_bit() was called?
Yeah, that's a bug. Thanks for fixing it.
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
<t...@kernel.org>
Fixes: 839a8e8660b6777e7fe4e80af1a048aebe2b5977
Reported-by: syzbot <syzbot+9873874c735f2892e...@syzkaller.appspotmail.com>
Signed-off-by: Jan Kara <j...@suse.cz>
---
fs/fs-writeback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fs-writeback.c b/fs/fs-wr
On Wed 02-05-18 12:45:40, Dave Chinner wrote:
> On Sat, Apr 21, 2018 at 03:03:09PM +0200, Jan Kara wrote:
> > On Wed 18-04-18 14:08:26, Dave Chinner wrote:
> > > From: Dave Chinner <dchin...@redhat.com>
> > >
> > > Currently iomap_dio_rw() only handles
se {
> + if (bdev->bd_disk != disk) {
> + ret = -ENXIO;
> + goto out_unlock_bdev;
> + }
> +
> if (bdev->bd_contains == bdev) {
> ret = 0;
> if (bdev->
and possibly some input from FS / MM / RDMA
folks about what might be acceptable.
Honza
[1] https://www.spinics.net/lists/linux-xfs/msg14468.html
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
would be on
BUG_ON(!buffer_mapped(bh));
but I'm not sure...
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
On Thu 01-02-18 19:58:45, Eric Biggers wrote:
> On Thu, Jan 11, 2018 at 06:00:08PM +0100, Jan Kara wrote:
> > On Thu 11-01-18 19:22:39, Hou Tao wrote:
> > > Hi,
> > >
> > > On 2018/1/11 16:24, Dan Carpenter wrote:
> > > > Thanks for your report and
reference. That way we make sure gendisk can
get associated only with visible bdev inodes.
Signed-off-by: Jan Kara <j...@suse.cz>
---
fs/block_dev.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index fe41a7
Add a proper counterpart to get_disk_and_module() -
put_disk_and_module(). Currently it is opencoded in several places.
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/blk-cgroup.c| 11 ++-
block/genhd.c | 20
fs/block_dev.c
Rename get_disk() to get_disk_and_module() to make sure what the
function does. It's not a great name but at least it is now clear that
put_disk() is not it's counterpart.
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/genhd.c | 10 --
drivers/block/amiflop.c | 2 +-
d
ath as if it is
completely run after new device is created).
Reported-and-analyzed-by: Hou Tao <hout...@huawei.com>
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/genhd.c | 21 -
include/linux/genhd.h | 1 +
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/
Hello,
these patches fix races happening when devices are frequently destroyed and
recreated in association of block device inode with corresponding gendisk.
Generally when such race happen it results in use-after-free issues, block
device page cache inconsistencies, or other problems. I have
s put_disk() is *not*
the counterpart of get_disk() but let's fix that in the follow up
commit since that will be more intrusive.
Fixes: 8ddcd653257c18a669fcb75ee42c37054908e0d6
CC: Christoph Hellwig <h...@lst.de>
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/genhd.c | 3 +++
1 file changed, 3 insertions(
Hello,
On Fri 02-02-18 15:04:11, Liu Bo wrote:
> On Thu, Jan 25, 2018 at 12:57:27PM +0100, Jan Kara wrote:
> > Hello,
> >
> > this is about a problem I have identified last month and for which I still
> > don't have good solution. Some discussion of the problem happe
On Tue 13-02-18 10:11:37, Hou Tao wrote:
> Hi Jan,
>
> On 2018/2/7 0:05, Jan Kara wrote:
> > When two blkdev_open() calls for a partition race with device removal
> > and recreation, we can hit BUG_ON(!bd_may_claim(bdev, whole, holder)) in
> > blkdev_open(). The
for driver lock;
> > (3) override driver lock with internal lock;
> > (4) unlock driver lock;
> > (5) can take driver lock now;
> > (6) but unlock internal lock.
> >
> > If we get blkg->q->queue_lock to local first like blk_cleanup_queue,
> >
On Mon 18-06-18 23:38:12, Tetsuo Handa wrote:
> On 2018/06/18 22:46, Jan Kara wrote:
> > syzbot is reporting NULL pointer dereference at wb_workfn() [1] due to
>
> [1]
> https://syzkaller.appspot.com/bug?id=e0818ccb7e46190b3f1038b0c794299208ed4206
>
> line is missing.
&
On Mon 18-06-18 10:40:14, Tejun Heo wrote:
> On Mon, Jun 18, 2018 at 03:46:58PM +0200, Jan Kara wrote:
> > syzbot is reporting NULL pointer dereference at wb_workfn() [1] due to
> > wb->bdi->dev being NULL. And Dmitry confirmed that wb->state was
> > WB_shutting_down
On Wed 22-08-18 18:50:53, Ming Lei wrote:
> On Wed, Aug 22, 2018 at 12:33:05PM +0200, Jan Kara wrote:
> > On Wed 22-08-18 10:02:49, Martin Wilck wrote:
> > > On Mon, 2018-07-30 at 20:37 +0800, Ming Lei wrote:
> > > > On Wed, Jul 25, 2018 at 11:15:
ADs as well?
>
> I haven't encountered this situation in my tests, and I'm unsure how to
> provoke it - run a direct IO test under high memory pressure?
Currently, iov_iter_get_pages() is always guaranteed to get at least one
page as that is current guarantee of get_user_pages() (unless we hit
EFAULT obviously). So bio_iov_iter_get_pages() as is now is guaranteed to
exhaust 'iter' or fill 'bio'. But in the future, the guarantee that
get_user_pages() will always pin at least one page may go away. But we'd
have to audit all users at that time anyway.
Honza
--
Jan Kara
SUSE Labs, CR
one as direct and part attempted to be done as
buffered. Also the "slow" direct IO path in __blkdev_direct_IO() behaves
differently - it aborts and returns error if bio_iov_iter_get_pages() ever
returned error. IMO we should do the same here.
Honza
--
Jan Kara
SUSE Labs, CR
s shifted by the value of bio->bi_vcnt at function
> invocation, the correct index is (nr_pages - 1).
>
> V2: improved readability following suggestions from Ming Lei.
>
> Fixes: 2cefe4dbaadf ("block: add bio_iov_iter_get_pages()")
> Signed-off-by: Martin Wilck
Loo
ork as expected?
Well, there has never been a promise that it will grab *all* pages in the
iter AFAIK. Practically, I think that it was just too hairy to implement in
the macro magic that iter processing is... Al might know more (added to
CC).
Honza
--
Jan Kara
SUSE Labs, CR
t;
>
> You might put the 'vecs' leak fix into another patch, and resue the
> current code block for that.
>
> Looks all users of bio_iov_iter_get_pages() need this kind of fix, so
> what do you think about the following way?
No. AFAICT all the other users of bio_iov_iter_get_pages() are perfectly
fine with it returning less pages and they loop appropriately.
Honza
--
Jan Kara
SUSE Labs, CR
rates. Martin optimized this to fill the bio
completely (as we know we have enough bvecs) before submitting which has
chances to perform better. I'm fine with either approach, just we have to
decide which way to go.
Honza
--
Jan Kara
SUSE Labs, CR
On Thu 19-07-18 14:23:53, Martin Wilck wrote:
> On Thu, 2018-07-19 at 12:45 +0200, Jan Kara wrote:
> > Secondly, I don't think it is good to discard error from
> > bio_iov_iter_get_pages() here and just submit partial IO. It will
> > again
> > lead to part of IO be
On Thu 19-07-18 20:20:51, Ming Lei wrote:
> On Thu, Jul 19, 2018 at 01:56:16PM +0200, Jan Kara wrote:
> > On Thu 19-07-18 19:04:46, Ming Lei wrote:
> > > On Thu, Jul 19, 2018 at 11:39:18AM +0200, Martin Wilck wrote:
> > > > bio_iov_iter_get_pages() returns onl
le
the second ends up doing read-modify-write cycle through page cache, the
first write could end up being lost. I'll try whether something like this
is able to see the corruption...
Honza
--
Jan Kara
SUSE Labs, CR
On Wed 18-07-18 13:40:07, Jan Kara wrote:
> On Wed 18-07-18 11:20:15, Johannes Thumshirn wrote:
> > On Wed, Jul 18, 2018 at 03:54:46PM +0800, Ming Lei wrote:
> > > Please go ahead and take care of it since you have the test cases.
> >
> > Speaking of which, do we al
0 RCX:
> 7fe63d6741f0
> [ 4677.112584] RDX: 0400 RSI: 55b1645c33e8 RDI:
> 000f
> [ 4677.121037] RBP: 55b1645c32b0 R08: 7fe63d65f008 R09:
> 0430
> [ 4677.129489] R10: 55b1645c33d8 R11: 0246 R12:
> 00000
Hello,
On Wed 11-04-18 08:11:13, Jens Axboe wrote:
> On 4/11/18 7:58 AM, Jan Kara wrote:
> > On Tue 10-04-18 11:17:46, Jens Axboe wrote:
> >> Been running some tests and I keep running into issues with hotplug.
> >> This looks similar to what Bart posted the other
seccomp_filter+0x28/0x230
> ? _raw_spin_unlock+0xa/0x20
> ? __handle_mm_fault+0x7d6/0x9b0
> ? list_lru_add+0xa8/0xc0
> ? _raw_spin_unlock+0xa/0x20
> ? __alloc_fd+0xaf/0x160
> ? do_sys_open+0x1a6/0x230
> do_sys_open+0x1a6/0x230
> do_syscall_6
ned long offset;
> unsigned long private;
> ...
> };
Auch, the fact that we could share a page as data storage for several
inode+offset combinations that are not sharing underlying storage just
looks viciously twisted ;) But is it really that useful to warrant
complications? In particular I'm afraid that filesystems expect consistency
between their internal state (attached to page->private) and page state
(e.g. page->flags) and when there are multiple internal states attached to
the same page this could go easily wrong...
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
On Tue 17-04-18 17:59:36, Luis R. Rodriguez wrote:
> On Thu, Dec 21, 2017 at 12:03:29PM +0100, Jan Kara wrote:
> > On Fri 01-12-17 22:13:27, Luis R. Rodriguez wrote:
> > >
> > > I'll note that its still not perfectly clear if really the semantics
> > > beh
o_end(), hence post-IO data stability updates will no
> long race against operations that serialise via inode_dio_wait()
> such as truncate or hole punch.
>
> Signed-Off-By: Dave Chinner <dchin...@redhat.com>
> Reviewed-by: Christoph Hellwig <h...@lst.de&
think this is quite correct. IOCB_DSYNC gets set also for O_SYNC
writes (in that case we also set IOCB_SYNC). And for those we cannot use
the FUA optimization AFAICT (definitely IOMAP_F_DIRTY isn't a safe
indicator of a need of full fsync for O_SYNC). Other than that the patch
looks good to me.
On Fri 20-04-18 11:49:32, Luis R. Rodriguez wrote:
> On Tue, Apr 17, 2018 at 05:59:36PM -0700, Luis R. Rodriguez wrote:
> > On Thu, Dec 21, 2017 at 12:03:29PM +0100, Jan Kara wrote:
> > > Hello,
> > >
> > > I think I owe you a reply here... Sorry that it
ng as lookup
> index. Usualy those flag are seldomly changed/accessed. Again the
> overhead (ignoring code size) would only be for page which are KSM.
> So maybe KSM will not make sense because perf overhead it has with
> page->flags access (i don't think so but i haven't tested this).
Yeah, sure, page->flags could be dealt with in a similar way but at this
point I don't think it's worth it. And without page->flags I don't think
abstracting page->private makes much sense - or am I missing something why
you need page->private depend on the mapping? So what I wanted to suggest
is that we leave page->private as is currently and just concentrate on
page->mapping hacks...
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
On Wed 25-04-18 00:07:07, Holger Hoffstätte wrote:
> On 04/24/18 19:34, Christoph Hellwig wrote:
> > On Sat, Apr 21, 2018 at 02:54:05PM +0200, Jan Kara wrote:
> > > > - if (iocb->ki_flags & IOCB_DSYNC)
> > > > +
On Fri 23-02-18 15:47:36, Mark Rutland wrote:
> Hi all,
>
> While fuzzing arm64/v4.16-rc2 with syzkaller, I simultaneously hit a
> number of splats in the block layer:
>
> * inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-R} usage in
> jbd2_trans_will_send_data_barrier
>
> * BUG: sleeping function
Rename get_disk() to get_disk_and_module() to make sure what the
function does. It's not a great name but at least it is now clear that
put_disk() is not it's counterpart.
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/genhd.c | 10 --
drivers/block/amiflop.c | 2 +-
d
s put_disk() is *not*
the counterpart of get_disk() but let's fix that in the follow up
commit since that will be more intrusive.
Fixes: 8ddcd653257c18a669fcb75ee42c37054908e0d6
CC: Christoph Hellwig <h...@lst.de>
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/genhd.c | 3 +++
1 file changed, 3 insertions(
reference. That way we make sure gendisk can
get associated only with visible bdev inodes.
Tested-by: Hou Tao <hout...@huawei.com>
Signed-off-by: Jan Kara <j...@suse.cz>
---
fs/block_dev.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/fs/bloc
ted-by: Hou Tao <hout...@huawei.com>
Signed-off-by: Jan Kara <j...@suse.cz>
---
fs/block_dev.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 1dbbf847911a..fe41a76769fa 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1
Add a proper counterpart to get_disk_and_module() -
put_disk_and_module(). Currently it is opencoded in several places.
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/blk-cgroup.c| 11 ++-
block/genhd.c | 20
fs/block_dev.c
Hello,
these patches fix races happening when devices are frequently destroyed and
recreated in association of block device inode with corresponding gendisk.
Generally when such race happen it results in use-after-free issues, block
device page cache inconsistencies, or other problems. I have
ath as if it is
completely run after new device is created).
Reported-and-analyzed-by: Hou Tao <hout...@huawei.com>
Tested-by: Hou Tao <hout...@huawei.com>
Signed-off-by: Jan Kara <j...@suse.cz>
---
block/genhd.c | 21 -
include/linux/genhd.h | 1 +
2 files changed
On Mon 26-02-18 11:38:19, Mark Rutland wrote:
> On Mon, Feb 26, 2018 at 11:52:56AM +0100, Jan Kara wrote:
> > On Fri 23-02-18 15:47:36, Mark Rutland wrote:
> > > Hi all,
> > >
> > > While fuzzing arm64/v4.16-rc2 with syzkaller, I simultaneously hit a
>
On Mon 26-02-18 09:04:40, Jens Axboe wrote:
> On 2/26/18 5:01 AM, Jan Kara wrote:
> > Hello,
> >
> > these patches fix races happening when devices are frequently destroyed and
> > recreated in association of block device inode with corresponding gendisk.
> >
they
either don't run udev or they feel not all their teams building new
products have enough expertise to come up with a proper set of rules...
Honza
--
Jan Kara
SUSE Labs, CR
On Tue 16-10-18 11:16:22, Omar Sandoval wrote:
> On Tue, Oct 16, 2018 at 01:36:54PM +0200, Jan Kara wrote:
> > On Wed 10-10-18 14:28:09, Jan Kara wrote:
> > > On Wed 10-10-18 13:42:27, Johannes Thumshirn wrote:
> > > > On Wed, Oct 10, 2018 at 07:19:00PM +0900, Tetsuo
On Wed 17-10-18 10:29:22, Jens Axboe wrote:
> On 10/17/18 4:05 AM, Jan Kara wrote:
> > On Tue 16-10-18 11:35:59, Jens Axboe wrote:
> >> On 10/15/18 1:44 PM, Paolo Valente wrote:
> >>> Here are some old results with a very simple configuration:
> >>> http:/
Hello,
these two patches create two new tests for blktests as regression tests
for my recently posted loopback device fixes. More details in individual
patches.
Honza
Add test for setting partscan flag.
Signed-off-by: Jan Kara
---
src/Makefile | 3 ++-
src/loop_set_status_partscan.c | 45 ++
tests/loop/006 | 33 +++
tests/loop/006.out | 2 ++
4
Add regression test for patch "block/loop: Use global lock for ioctl()
operation." where we can oops while traversing list of loop devices
backing newly created device.
Signed-off-by: Jan Kara
---
src/Makefile | 3 ++-
src/loop_change_fd.c | 48 ++
On Wed 10-10-18 14:28:09, Jan Kara wrote:
> On Wed 10-10-18 13:42:27, Johannes Thumshirn wrote:
> > On Wed, Oct 10, 2018 at 07:19:00PM +0900, Tetsuo Handa wrote:
> > > On 2018/10/10 19:04, Jan Kara wrote:
> > > > Hi,
> > > >
> > > > this patc
On Mon 22-10-18 15:52:55, Omar Sandoval wrote:
> On Thu, Oct 18, 2018 at 12:31:46PM +0200, Jan Kara wrote:
> > Add test for setting partscan flag.
> >
> > Signed-off-by: Jan Kara
>
> Sorry I didn't notice this earlier, but loop/001 already does a
> partition
On Tue 23-10-18 11:57:50, Omar Sandoval wrote:
> On Tue, Oct 23, 2018 at 12:05:12PM +0200, Jan Kara wrote:
> > On Mon 22-10-18 15:52:55, Omar Sandoval wrote:
> > > On Thu, Oct 18, 2018 at 12:31:46PM +0200, Jan Kara wrote:
> > > > Add test for setting partscan fla
Move setting of lo_state to Lo_rundown out into the callers. That will
allow us to unlock loop_ctl_mutex while the loop device is protected
from other changes by its special state.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 52 +++-
1 file
roperly fixed, let's stop fooling lockdep.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 112afc9bc604..bf6bc35aaf88 100644
--- a/drivers/block/loop.c
+++ b/drivers
Push loop_ctl_mutex down to loop_change_fd(). We will need this to be
able to call loop_reread_partitions() without loop_ctl_mutex.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/block/loop.c
Push loop_ctl_mutex down to loop_set_status(). We will need this to be
able to call loop_reread_partitions() without loop_ctl_mutex.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 51 +--
1 file changed, 25 insertions(+), 26 deletions(-)
diff
fix deadlock possibility.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/
kdep warning and the possible
deadlock.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/d
Now that loop_ctl_mutex is global, just get rid of loop_index_mutex as
there is no good reason to keep these two separate and it just
complicates the locking.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 41 -
1 file changed, 20 insertions(+), 21
Hi,
this patch series fixes oops and possible deadlocks as reported by syzbot [1]
[2]. The second patch in the series (from Tetsuo) fixes the oops, the remaining
patches are cleaning up the locking in the loop driver so that we can in the
end reasonably easily switch to rereading partitions
suo Handa
Reported-by: syzbot
Reviewed-by: Jan Kara
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 58 ++--
drivers/block/loop.h | 1 -
2 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loo
From: Tetsuo Handa
vfs_getattr() needs "struct path" rather than "struct file".
Let's use path_get()/path_put() rather than get_file()/fput().
Signed-off-by: Tetsuo Handa
Reviewed-by: Jan Kara
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 10 +-
1 file c
reference until we have released loop_ctl_mutex.
Reported-by: Tetsuo Handa
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index b3f981ac8ef1..112afc9bc604 100644
Push loop_ctl_mutex down to loop_get_status() to avoid the unusual
convention that the function gets called with loop_ctl_mutex held and
releases it.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 37 ++---
1 file changed, 10 insertions(+), 27 deletions
loop_clr_fd() has a weird locking convention that is expects
loop_ctl_mutex held, releases it on success and keeps it on failure.
Untangle the mess by moving locking of loop_ctl_mutex into
loop_clr_fd().
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 49
Push lo_ctl_mutex down to loop_set_fd(). We will need this to be able to
call loop_reread_partitions() without lo_ctl_mutex.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/block/loop.c b
__loop_release() has a single call site. Fold it there. This is
currently not a huge win but it will make following replacement of
loop_index_mutex more obvious.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git
special handling to reduce unnecessary code duplication.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 88 +---
1 file changed, 63 insertions(+), 25 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index dcdc96f4d2d4
because we use only lo->lo_number and lo->lo_file_name in case of error
for reporting purposes (thus possibly reporting outdate information is
not a big deal) and we are safe from 'lo' going away under us by
elevated lo->lo_refcnt.
Signed-off-by: Jan Kara
---
drivers/block/lo
On Thu 08-11-18 06:21:21, Jens Axboe wrote:
> On 11/8/18 6:01 AM, Jan Kara wrote:
> > Hi,
> >
> > this patch series fixes oops and possible deadlocks as reported by syzbot
> > [1]
> > [2]. The second patch in the series (from Tetsuo) fixes the oops, the
> >
On Thu 08-11-18 16:28:11, Theodore Y. Ts'o wrote:
> On Thu, Nov 08, 2018 at 02:01:00PM +0100, Jan Kara wrote:
> > Hi,
> >
> > this patch series fixes oops and possible deadlocks as reported by syzbot
> > [1]
> > [2]. The second patch in the series (from Tetsuo) f
because we use only lo->lo_number and lo->lo_file_name in case of error
for reporting purposes (thus possibly reporting outdate information is
not a big deal) and we are safe from 'lo' going away under us by
elevated lo->lo_refcnt.
Signed-off-by: Jan Kara
---
drivers/block/lo
Now that loop_ctl_mutex is global, just get rid of loop_index_mutex as
there is no good reason to keep these two separate and it just
complicates the locking.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 41 -
1 file changed, 20 insertions(+), 21
suo Handa
Reported-by: syzbot
Reviewed-by: Jan Kara
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 58 ++--
drivers/block/loop.h | 1 -
2 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loo
From: Tetsuo Handa
vfs_getattr() needs "struct path" rather than "struct file".
Let's use path_get()/path_put() rather than get_file()/fput().
Signed-off-by: Tetsuo Handa
Reviewed-by: Jan Kara
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 10 +-
1 file c
Push loop_ctl_mutex down to loop_change_fd(). We will need this to be
able to call loop_reread_partitions() without loop_ctl_mutex.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/block/loop.c
__loop_release() has a single call site. Fold it there. This is
currently not a huge win but it will make following replacement of
loop_index_mutex more obvious.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git
special handling to reduce unnecessary code duplication.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 88 +---
1 file changed, 63 insertions(+), 25 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 3fa5e63944a4
Push lo_ctl_mutex down to loop_set_fd(). We will need this to be able to
call loop_reread_partitions() without lo_ctl_mutex.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/block/loop.c b
Push loop_ctl_mutex down to loop_set_status(). We will need this to be
able to call loop_reread_partitions() without loop_ctl_mutex.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 51 +--
1 file changed, 25 insertions(+), 26 deletions(-)
diff
kdep warning and the possible
deadlock.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/d
loop_clr_fd() has a weird locking convention that is expects
loop_ctl_mutex held, releases it on success and keeps it on failure.
Untangle the mess by moving locking of loop_ctl_mutex into
loop_clr_fd().
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 49
Move setting of lo_state to Lo_rundown out into the callers. That will
allow us to unlock loop_ctl_mutex while the loop device is protected
from other changes by its special state.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 52 +++-
1 file
reference until we have released loop_ctl_mutex.
Reported-by: Tetsuo Handa
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index d6b3fac25040..a517247a32fa 100644
Push loop_ctl_mutex down to loop_get_status() to avoid the unusual
convention that the function gets called with loop_ctl_mutex held and
releases it.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 37 ++---
1 file changed, 10 insertions(+), 27 deletions
Hi,
this patch series fixes oops and possible deadlocks as reported by syzbot [1]
[2]. The second patch in the series (from Tetsuo) fixes the oops, the remaining
patches are cleaning up the locking in the loop driver so that we can in the
end reasonably easily switch to rereading partitions
On Wed 10-10-18 13:42:27, Johannes Thumshirn wrote:
> On Wed, Oct 10, 2018 at 07:19:00PM +0900, Tetsuo Handa wrote:
> > On 2018/10/10 19:04, Jan Kara wrote:
> > > Hi,
> > >
> > > this patch series fixes oops and possible deadlocks as reported by syzbot
&
.
This is deliberate so that we get rid of the weird "__loop_clr_fd()
releases mutex it did not acquire". This is not performance critical path
by any means so better keep the locking simple.
Honza
--
Jan Kara
SUSE Labs, CR
oks good to me. Feel free
to add:
Reviewed-by: Jan Kara
Honza
> ---
> drivers/block/loop.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
partitions() and reorganizing the code there so
that loop_reread_partitions() is called as late as possible so that it is
clear that dropping the mutex is fine (and usually we don't even have to
reacquire it). Plus your patch does not seem to take care of the possible
races of loop_clr_fd() with LOOP_CTL_REMOVE? See my other mail for
details...
Honza
--
Jan Kara
SUSE Labs, CR
nt ret = -ENOSYS;
> + int ret = lock_loop_killable();
>
> - mutex_lock(_index_mutex);
> + if (ret)
> + return ret;
> + ret = -ENOSYS;
> switch (cmd) {
> case LOOP_CTL_ADD:
> ret = loop_lookup(, parm);
> @@ -2019,21 +2040,15 @@ static long loop_control_ioctl(struct file *file,
> unsigned int cmd,
> ret = loop_lookup(, parm);
> if (ret < 0)
> break;
> - ret = mutex_lock_killable(_ctl_mutex);
> - if (ret)
> - break;
> if (lo->lo_state != Lo_unbound) {
> ret = -EBUSY;
> - mutex_unlock(_ctl_mutex);
> break;
> }
> if (atomic_read(>lo_refcnt) > 0) {
> ret = -EBUSY;
> - mutex_unlock(_ctl_mutex);
> break;
> }
> lo->lo_disk->private_data = NULL;
> - mutex_unlock(_ctl_mutex);
> idr_remove(_index_idr, lo->lo_number);
> loop_remove(lo);
> break;
> @@ -2043,7 +2058,7 @@ static long loop_control_ioctl(struct file *file,
> unsigned int cmd,
> break;
> ret = loop_add(, -1);
> }
> - mutex_unlock(_index_mutex);
> + unlock_loop();
>
> return ret;
> }
> @@ -2127,10 +2142,10 @@ static int __init loop_init(void)
> THIS_MODULE, loop_probe, NULL, NULL);
>
> /* pre-create number of devices given by config or max_loop */
> - mutex_lock(_index_mutex);
> + lock_loop();
> for (i = 0; i < nr; i++)
> loop_add(, i);
> - mutex_unlock(_index_mutex);
> + unlock_loop();
>
> printk(KERN_INFO "loop: module loaded\n");
> return 0;
> --
> 1.8.3.1
>
--
Jan Kara
SUSE Labs, CR
301 - 400 of 417 matches
Mail list logo
