All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jun'ichi Nomura" <j-nomura@ce.jp.nec.com>
To: Mike Snitzer <snitzer@redhat.com>
Cc: device-mapper development <dm-devel@redhat.com>
Subject: Re: dm: use revalidate_disk to update device size after set_capacity
Date: Fri, 29 Oct 2010 12:00:10 +0900	[thread overview]
Message-ID: <4CCA38BA.3070406@ce.jp.nec.com> (raw)
In-Reply-To: <20101028195432.GA23063@redhat.com>

Hi Mike,

(10/29/10 04:54), Mike Snitzer wrote:
>> If __set_size() could be done in later stage of do_resume(),
>> we can use revalidate_disk() for dm, too.
>> What do you think?
> 
> I think it would be incorrect to make a new DM table live without first
> adjusting the size of the DM device to reflect the new table.  Could
> result in racing IO to the end of a device which would cause IO errors
> like "access beyond end of device".

Yes, but at that point, resume ioctl is still in progress.
Doesn't that mean "the resize is still in progress"?
If so, who cares that I/O while the resize is not yet completed?

> I'll cleanup the patch from my previous mail to include some in-code
> comments and re-post it.  Please ack it if you agree that direct use of
> bd_mutex in __set_size() is a reasonable fix given the current code.

My point is that it's better to avoid having a code with
special assumption on generic code, either "any codes don't do
I/O while holding bd_mutex" or "dm_resume is the only code
which calls i_size_write() for dm device".
I think it's prone for someone who don't care dm specifics
to introduce a new code that breaks such an assumption.

Anyway, I think your bd_mutex patch should be fine for now and
is better than the current code with i_mutex, which has a real deadlock issue.

-- 
Jun'ichi Nomura, NEC Corporation

  parent reply	other threads:[~2010-10-29  3:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-19 22:07 [PATCH] dm: use revalidate_disk to update device size after set_capacity Mike Snitzer
2010-10-20  5:42 ` Jun'ichi Nomura
2010-10-28  1:16   ` Mike Snitzer
2010-10-28 12:15     ` Jun'ichi Nomura
2010-10-28 19:54       ` Mike Snitzer
2010-10-28 22:18         ` Mike Snitzer
2010-10-29  3:00         ` Jun'ichi Nomura [this message]
2010-10-29 21:50 ` dm: lock bd_mutex when setting device size Mike Snitzer
2010-11-01  7:19   ` Jun'ichi Nomura
2010-11-01 13:14     ` Mike Snitzer
2010-11-03 18:06       ` [PATCH] dm: remove extra locking when changing " Mike Snitzer

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=4CCA38BA.3070406@ce.jp.nec.com \
    --to=j-nomura@ce.jp.nec.com \
    --cc=dm-devel@redhat.com \
    --cc=snitzer@redhat.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.