| Message ID | 1458627149-12988-8-git-send-email-ming.lei@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 22, 2016 at 02:12:28PM +0800, Ming Lei wrote: > drbd is the only user of BIO_MAX_SIZE, so use BIO_MAX_PAGES > instead. That whole code block looks completely bogus to me, although your patch doesn't make it any worse. I/O size for a network protocol shouldn't dependend on the number of vectors in a kernel internal structure. Well, getting rid of BIO_MAX_SIZE is worth it, so: Reviewed-by: Christoph Hellwig <hch@lst.de> -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 29, 2016 at 12:31:24AM -0700, Christoph Hellwig wrote: > On Tue, Mar 22, 2016 at 02:12:28PM +0800, Ming Lei wrote: > > drbd is the only user of BIO_MAX_SIZE, so use BIO_MAX_PAGES > > instead. > > That whole code block looks completely bogus to me, although your patch > doesn't make it any worse. > > I/O size for a network protocol shouldn't dependend on the number of > vectors in a kernel internal structure. That's correct. But we needed some limit there. Initially, up until I changed it like six years ago iirc, the receiving side would receive into a single bio. So limiting us to what a single bio could usually handle seemed like a good idea at the time. Today, we should be able to handle 128 MiB easily, maybe more. But that would require a protocol bump to stay backwards compatible. The part about "architecture not supported", if our limit (1 MiB) is bigger than the "system" limit: Never met that in real life. Probably not even possible. Just a paranoia on my side: what if. If that would have happened somewhere, on some strange architecture or configuration, I wanted to know about that. Best way: don't even compile. > Well, getting rid of BIO_MAX_SIZE is worth it, so: > > Reviewed-by: Christoph Hellwig <hch@lst.de> Thanks, Lars Ellenberg -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h index c227fd4..10bfff1 100644 --- a/drivers/block/drbd/drbd_int.h +++ b/drivers/block/drbd/drbd_int.h @@ -1327,14 +1327,14 @@ struct bm_extent { #endif #endif -/* BIO_MAX_SIZE is 256 * PAGE_CACHE_SIZE, +/* Estimate max bio size as 256 * PAGE_CACHE_SIZE, * so for typical PAGE_CACHE_SIZE of 4k, that is (1<<20) Byte. * Since we may live in a mixed-platform cluster, * we limit us to a platform agnostic constant here for now. * A followup commit may allow even bigger BIO sizes, * once we thought that through. */ #define DRBD_MAX_BIO_SIZE (1U << 20) -#if DRBD_MAX_BIO_SIZE > BIO_MAX_SIZE +#if DRBD_MAX_BIO_SIZE > (BIO_MAX_PAGES << PAGE_CACHE_SHIFT) #error Architecture not supported: DRBD_MAX_BIO_SIZE > BIO_MAX_SIZE #endif #define DRBD_MAX_BIO_SIZE_SAFE (1U << 12) /* Works always = 4k */
drbd is the only user of BIO_MAX_SIZE, so use BIO_MAX_PAGES instead. Signed-off-by: Ming Lei <ming.lei@canonical.com> --- drivers/block/drbd/drbd_int.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)