LKML Archive on
help / color / mirror / Atom feed
From: Christoph Hellwig <>
To: Tejun Heo <>
Cc: David Herrmann <>,, Jens Axboe <>,
	Andrew Morton <>,, Al Viro <>
Subject: Re: [PATCH RFC] loop: make partition scanning reliable
Date: Mon, 26 Jan 2015 09:27:06 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Mon, Jan 26, 2015 at 11:13:28AM -0500, Tejun Heo wrote:
> > @@ -159,12 +159,15 @@ static int blkdev_reread_part(struct block_device *bdev)
> >  		return -EINVAL;
> >  	if (!capable(CAP_SYS_ADMIN))
> >  		return -EACCES;
> > -	if (!mutex_trylock(&bdev->bd_mutex))
> > +	if (!skipbusy)
> > +		mutex_lock(&bdev->bd_mutex);
> > +	else if (!mutex_trylock(&bdev->bd_mutex))
> >  		return -EBUSY;
> Do we actually need the mutex_trylock() path?  Why can't we just
> always grab the mutex?

I wondered about this, too.  The trylock goes all the way back
to when Al first factored out the partition re-reading into common
code ("[PATCH] partition table flush/read cleanup" in the historic tree
converted from bk), and even before that most drivers used a weird
dev_lock_part() consruct operating on a kdev_t, wich looked like a

Given that blkdev_reread_part is invoked directly from the ioctl method
without any additional locking it seems fairly pointless for the
case where it's issue from userspace.  In addition to that the loop
driver, nbd and dasd issue it from a driver, as does the root mounting
code for md.

I would be surprised if any of these callers needs it, but a careful
audit would be useful.

I'd also really prefer a patch that makes loop, nbd and dasd call
blkdev_reread_part directly instead of by ioctl_by_bdev as a first
stage preparation, followed by the locking changes  and the changes
to loop.c in separate patch so each does one thing and can be properly

      reply	other threads:[~2015-01-26 17:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-26 10:15 David Herrmann
2015-01-26 16:13 ` Tejun Heo
2015-01-26 17:27   ` Christoph Hellwig [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH RFC] loop: make partition scanning reliable' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).