linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: snitzer@redhat.com (Mike Snitzer)
Subject: [PATCH 5/5] block: Inline blk_integrity in struct gendisk
Date: Tue, 15 Sep 2015 21:07:21 -0400	[thread overview]
Message-ID: <20150916010720.GA21398@redhat.com> (raw)
In-Reply-To: <1440103314-31610-6-git-send-email-martin.petersen@oracle.com>

On Thu, Aug 20 2015 at  4:41pm -0400,
Martin K. Petersen <martin.petersen@oracle.com> wrote:

> Up until now the_integrity profile has been dynamically allocated and
> attached to struct gendisk after the disk has been made active.
> 
> This causes problems because NVMe devices need to register the profile
> prior to the partition table being read due to a mandatory metadata
> buffer requirement. In addition, DM goes through hoops to deal with
> preallocating, but not initializing integrity profiles.

Yes, but only if the underlying device(s) actually support bip.  This
change to inlining blk_integrity (purely to make NVMe happy) comes at
the cost of _always_ allocating the memory 'struct blk_integrity' (via
gendisk inline) even if there is absolutely no need for it.

That said, with your changes the blk_integrity structure is no longer
large (previously was 96 bytes).. so I can let that part go.

> Since the integrity profile is small (4 bytes + a pointer), Christoph
> suggested moving it to struct gendisk proper. This requires several
> changes:
> 
>  - Moving the blk_integrity definition to genhd.h.
> 
>  - Inlining blk_integrity in struct gendisk.
> 
>  - Removing the dynamic allocation code.
> 
>  - Adding helper functions which allow gendisk to set up and tear down
>    the integrity sysfs dir when a disk is added/deleted.
> 
>  - Adding a blk_integrity_revalidate() callback for updating the stable
>    pages bdi setting.
> 
>  - The calls that depend on whether a device has an integrity profile or
>    not now key off of the bi->profile pointer.
> 
>  - Simplifying the integrity support routines in DM.

But I cannot let this DM "simplifying" go (vast majority of it breaks
bip for DM).  You missed the fact that DM has inactive and active
tables.  And that the top-level DM device can only be changed once an
inactive table is made active via DM's resume.  The bip for the DM
device cannot be changed during table load.  Such a change can only be
done during table resume.  Reason is an inactive DM table can get thrown
away at any time; so changing the top-level DM device during the load of
an inactive table isn't right.

Please review the commit header from commit a63a5cf84 ("dm: improve
block integrity support").

Long story short, DM changes that eliminate the checks/code for
allocating the blk_integrity structure should be all that is needed for
this patch.

I'll work through this further.  Hope to send an incremental patch that
fixes things up in the next day or so.

  parent reply	other threads:[~2015-09-16  1:07 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-14 17:57 [PATCH] NVMe: Reread partitions on metadata formats Keith Busch
2015-07-14 18:49 ` Jens Axboe
2015-07-14 19:01   ` Paul Grabinar
2015-07-15  3:48   ` Dan Williams
2015-07-15 21:36     ` Jens Axboe
2015-07-15 22:28       ` Keith Busch
2015-07-16  9:19         ` Christoph Hellwig
2015-07-17  1:47           ` Martin K. Petersen
2015-07-17  9:30             ` Christoph Hellwig
2015-07-21  6:02           ` Data integrity tweaks Martin K. Petersen
2015-07-21  6:02             ` [PATCH 1/5] block: Move integrity kobject to struct gendisk Martin K. Petersen
2015-07-22 11:32               ` Sagi Grimberg
2015-07-21  6:02             ` [PATCH 2/5] block: Consolidate static integrity profile properties Martin K. Petersen
2015-07-22 11:33               ` Sagi Grimberg
2015-07-21  6:02             ` [PATCH 3/5] block: Reduce the size of struct blk_integrity Martin K. Petersen
2015-07-21 11:53               ` Christoph Hellwig
2015-07-24 15:14                 ` Martin K. Petersen
2015-07-22 11:35               ` Sagi Grimberg
2015-07-21  6:02             ` [PATCH 4/5] block: Export integrity data interval size in sysfs Martin K. Petersen
2015-07-22 11:37               ` Sagi Grimberg
2015-07-24 15:26                 ` Martin K. Petersen
2015-07-21  6:02             ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Martin K. Petersen
2015-07-21 12:01               ` Christoph Hellwig
2015-08-20 20:41                 ` Simplify block integrity registration Martin K. Petersen
2015-08-20 20:41                   ` [PATCH 1/5] block: Move integrity kobject to struct gendisk Martin K. Petersen
2015-09-16 17:26                     ` Mike Snitzer
2015-08-20 20:41                   ` [PATCH 2/5] block: Consolidate static integrity profile properties Martin K. Petersen
2015-08-20 20:41                   ` [PATCH 3/5] block: Reduce the size of struct blk_integrity Martin K. Petersen
2015-08-20 20:41                   ` [PATCH 4/5] block: Export integrity data interval size in sysfs Martin K. Petersen
2015-08-20 20:41                   ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Martin K. Petersen
2015-08-21 23:47                     ` Busch, Keith
2015-08-27  0:25                       ` Martin K. Petersen
2015-08-27  0:25                       ` [PATCH] " Martin K. Petersen
2015-08-27  8:28                         ` Christoph Hellwig
2015-09-03 20:38                         ` Keith Busch
2015-09-04  3:25                           ` Martin K. Petersen
2015-10-12 21:05                       ` Block integrity registration update Martin K. Petersen
2015-10-12 21:05                         ` [PATCH 1/5] block: Move integrity kobject to struct gendisk Martin K. Petersen
2015-10-12 21:05                         ` [PATCH 2/5] block: Consolidate static integrity profile properties Martin K. Petersen
2015-10-14  1:11                           ` Dan Williams
2015-10-14  7:23                             ` Christoph Hellwig
2015-10-14 19:42                               ` Williams, Dan J
2015-10-14 19:47                                 ` hch
2015-10-14 20:00                                   ` Williams, Dan J
2015-10-14 22:42                                     ` Martin K. Petersen
2015-10-12 21:05                         ` [PATCH 3/5] block: Reduce the size of struct blk_integrity Martin K. Petersen
2015-10-12 21:05                         ` [PATCH 4/5] block: Export integrity data interval size in sysfs Martin K. Petersen
2015-10-12 21:05                         ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Martin K. Petersen
2015-10-12 23:06                           ` Mike Snitzer
2015-10-13  0:31                         ` Block integrity registration update Williams, Dan J
2015-10-13  1:53                           ` Williams, Dan J
2015-10-13 12:26                             ` hch
2015-10-13 17:38                               ` Dan Williams
2015-09-16  1:07                     ` Mike Snitzer [this message]
2015-09-21 20:45                       ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Mike Snitzer
2015-10-09  7:36                         ` Christoph Hellwig
2015-10-12  1:17                           ` Martin K. Petersen
2015-08-20 20:45                   ` Simplify block integrity registration Mike Snitzer
2015-07-17  1:44         ` [PATCH] NVMe: Reread partitions on metadata formats Martin K. Petersen
2015-07-15 17:37   ` Paul Grabinar
  -- strict thread matches above, loose matches on Subject: below --
2015-10-20  2:24 [PATCH v2 10/12] block: move blk_integrity to request_queue Martin K. Petersen
2015-10-20  2:45 ` Simplify block integrity registration v2 Martin K. Petersen
2015-10-20  2:45   ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Martin K. Petersen

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=20150916010720.GA21398@redhat.com \
    --to=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 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).