linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: snitzer@redhat.com (Mike Snitzer)
Subject: [PATCH 1/5] block: Move integrity kobject to struct gendisk
Date: Wed, 16 Sep 2015 13:26:01 -0400	[thread overview]
Message-ID: <20150916172601.GA25973@redhat.com> (raw)
In-Reply-To: <1440103314-31610-2-git-send-email-martin.petersen@oracle.com>

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

> The integrity kobject purely exists to support the integrity
> subdirectory in sysfs and doesn't really have anything to do with the
> blk_integrity data structure. Move the kobject to struct gendisk where
> it belongs.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
> Reported-by: Christoph Hellwig <hch at lst.de>
> Reviewed-by: Sagi Grimberg <sagig at mellanox.com>

[I understand both Martin and Keith are on vacation but I'm putting the
time to this now with the understanding that a proper exchange may be
delayed.]

Thinking about this series further: I don't really agree with it.
Maybe in the end the benefit of embedding in gendisk outweighs the
complexity of dynamic allocation... but I need to see it for myself.

Christoph, if you _know_ this to be the right way forward I can accept
it but please elaborate further.

What we currently have (before this patchset) has the very real benefit
of not wasting any memory if the block device doesn't have integrity
(DIF/DIX) support.  Especially given how rare bip supporting devices
still are.  Is there reason to expect they won't be so rare in the
future?  Every NVMe and nvdimm device will have integrity support?

This patch moves thes 64 byte 'struct kobject' out into 'struct
gendisk'.  I'm not seeing why (on x86_64 anyway) we'd what to always
allocate an extra 76 bytes (blk_integrity + kobject) for _every_ gendisk
regardless of whether a bip is needed..

76 bytes may not sound like a lot but in the context of DM-mpath that
adds up when you have systems with 1000s of devices with multiple paths
and then a DM mpath device (with its own gendisk) ontop -- 1000 devices
with 4 paths can waste 5000 * 76bytes = ~372K (even 372K isn't much...)

If DM-core could support existing dynamic bip (albeit with more involved
code born out of DM-specific quirks of device stacking and inactive and
active tables) then I have to believe NVMe can too.

As such I'm now going to shift my focus to revisiting what is so hard
for NVMe that it cannot cope with the existing dynamic allocation of
struct blk_integrity.

[If that turns out to be a waste of time (and NVMe/nvdimm/whatever would
be needlessly inconvenienced) then I'll switch back to fixing DM to cope
with this change.  But to be clear the current proposed DM changes
_cannot_ go upstream.]

  reply	other threads:[~2015-09-16 17:26 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 [this message]
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                     ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Mike Snitzer
2015-09-21 20:45                       ` 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 1/5] block: Move integrity kobject to 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=20150916172601.GA25973@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).