All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Phillip Susi <psusi@ubuntu.com>
Cc: Maxim Patlasov <maxim.patlasov@gmail.com>,
	joe@perches.com, kzak@redhat.com, linux-kernel@vger.kernel.org,
	jaxboe@fusionio.com
Subject: Re: [PATCH 1/2] Add partition resize function to BLKPG ioctl
Date: Thu, 26 Jan 2012 16:04:56 -0500	[thread overview]
Message-ID: <20120126210456.GC11297@redhat.com> (raw)
In-Reply-To: <4F21B91E.90106@ubuntu.com>

On Thu, Jan 26, 2012 at 03:35:42PM -0500, Phillip Susi wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 1/26/2012 2:01 PM, Vivek Goyal wrote:
> > I thought update will always happen with mutex lock held. That's
> > what sequence counter expects so that two updaters don't race. Just
> > that while updating under mutex lock, we still need to use sequence
> > counter mecahinsm to update values so that any readers out there
> > not holding mutex don't get confused.
> 
> Yes, but holding the mutex while writing does no good for the reader.
>  When the writer doesn't use the seqcounter, then the reader that is
> using it is not actually protected.

Ok, so you are worried about updates to nr_sects by other code. Makes
sense.

[..]
> > Are you still pursuing this pathset? Sounds like a useful
> > functionality to have.
> 
> Yes, but I hadn't yet heard back about my question about this being a
> broader issue that is already a bug in the kernel because things like
> loop and md already change nr_sects ( on the whole disk partition )
> without any protection.

Interesting. I do see that set_capacity() changes the nr_sects without
any protection. Sounds like it is racy on 32bit machines with 64bit
sector_t.

> 
> Maybe what we need is a read/write lock on struct genhd, then all
> readers need to acquire the read lock, which should only slow them
> down if they collide with a writer.

But taking lock will mean atomic operation irrespective of the fact
whether lock is taken by somebody else or not. I am assuming it will
still turn out to be expensive.

> 
> Another idea that I had but have not yet checked to see if it is
> actually feasible is to copy the struct genhd, change the size of the
> copy, and replace the existing one since updating the pointer will be
> atomic.

You will run into issues if somebody has a pointer stored to genhd.

I think simpler thing would be to stick with sequence counter approach
which keeps read side lockless. We can fix other writers of nr_sects
over a period of time. If nobody has complained so far, that means
we don't run into issues frequently and it is not a huge concern.

Thanks
Vivek

  reply	other threads:[~2012-01-26 21:05 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-29 17:06 [PATCH 1/2] Add partition resize function to BLKPG ioctl Maxim Patlasov
2011-12-30  0:09 ` Phillip Susi
2012-01-01 21:49   ` Phillip Susi
2012-01-26 19:01   ` Vivek Goyal
2012-01-26 20:35     ` Phillip Susi
2012-01-26 21:04       ` Vivek Goyal [this message]
2012-01-26 21:48         ` Phillip Susi
2012-01-30 15:49           ` Vivek Goyal
     [not found] <cover.1322709471.git.psusi@cfl.rr.com>
2011-12-01  3:23 ` Phillip Susi
2011-12-08 12:30   ` Karel Zak
2011-12-08 14:22     ` Phillip Susi
2011-12-08 15:16       ` Karel Zak
2011-12-08 15:25         ` Phillip Susi
2011-12-08 15:58           ` Vivek Goyal
2011-12-08 16:06             ` Phillip Susi
2011-12-08 16:28               ` Vivek Goyal
2011-12-08 16:55                 ` Phillip Susi
2011-12-09  2:53                 ` Phillip Susi
2011-12-12 14:53                   ` Vivek Goyal
2011-12-12 17:43                     ` Phillip Susi
2011-12-12 17:49                       ` Joe Perches
2011-12-12 18:04                         ` Vivek Goyal
2011-12-13  0:15                           ` Phillip Susi
2011-12-13  0:16                             ` Phillip Susi
2011-12-19 20:25                               ` Vivek Goyal
2011-12-21  1:53                                 ` Phillip Susi
2011-12-21  1:54                                   ` Phillip Susi
2011-12-21 20:46                                   ` Vivek Goyal
2011-12-24 21:36                                     ` Phillip Susi
2011-12-24 22:21                                       ` Phillip Susi

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20120126210456.GC11297@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=jaxboe@fusionio.com \
    --cc=joe@perches.com \
    --cc=kzak@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxim.patlasov@gmail.com \
    --cc=psusi@ubuntu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.