diff mbox

[RFC] blk: increase logical_block_size to unsigned int

Message ID 1483938267-8858-1-git-send-email-minchan@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Minchan Kim Jan. 9, 2017, 5:04 a.m. UTC
Mostly, zram is used as swap system on embedded world so it want to do IO
as PAGE_SIZE aligned/size IO unit. For that, one of the problem was
blk_queue_logical_block_size(zram->disk->queue, PAGE_SIZE) made overflow
in *64K page system* so [1] changed it to constant 4096.
Since that, partial IO can happen so zram should handle it which makes zram
complicated[2].

Now, I want to remove that partial IO handling logics in zram.

Block guys, Robert, Jerome:

Can't we extend q->limits.logical_block_size to unsigned int?
Is there any problem on that?

[1] 7b19b8d45b21, zram: Prevent overflow in logical block size
[2] 924bd88d703e, zram: allow partial page operations

Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Robert Jennings <rcj@linux.vnet.ibm.com>
Cc: Jerome Marchand <jmarchan@redhat.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 block/blk-settings.c   |  2 +-
 include/linux/blkdev.h | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Sergey Senozhatsky Jan. 9, 2017, 2:33 p.m. UTC | #1
On (01/09/17 14:04), Minchan Kim wrote:
> Mostly, zram is used as swap system on embedded world so it want to do IO
> as PAGE_SIZE aligned/size IO unit. For that, one of the problem was
> blk_queue_logical_block_size(zram->disk->queue, PAGE_SIZE) made overflow
> in *64K page system* so [1] changed it to constant 4096.
> Since that, partial IO can happen so zram should handle it which makes zram
> complicated[2].
> 

I thought that zram partial IO support is there because some file
systems cannot cope with large logical_block_size. like FAT, for
example. am I wrong?

	-ss
--
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
Jerome Marchand Jan. 9, 2017, 3:09 p.m. UTC | #2
----- Original Message -----
> From: "Sergey Senozhatsky" <sergey.senozhatsky@gmail.com>
> To: "Minchan Kim" <minchan@kernel.org>
> Cc: "Jens Axboe" <axboe@kernel.dk>, "Hyeoncheol Lee" <cheol.lee@lge.com>, linux-block@vger.kernel.org,
> linux-kernel@vger.kernel.org, "Andrew Morton" <akpm@linux-foundation.org>, "Sergey Senozhatsky"
> <sergey.senozhatsky@gmail.com>, "Robert Jennings" <rcj@linux.vnet.ibm.com>, "Jerome Marchand" <jmarchan@redhat.com>
> Sent: Monday, January 9, 2017 3:33:44 PM
> Subject: Re: [RFC] blk: increase logical_block_size to unsigned int
> 
> On (01/09/17 14:04), Minchan Kim wrote:
> > Mostly, zram is used as swap system on embedded world so it want to do IO
> > as PAGE_SIZE aligned/size IO unit. For that, one of the problem was
> > blk_queue_logical_block_size(zram->disk->queue, PAGE_SIZE) made overflow
> > in *64K page system* so [1] changed it to constant 4096.
> > Since that, partial IO can happen so zram should handle it which makes zram
> > complicated[2].
> > 
> 
> I thought that zram partial IO support is there because some file
> systems cannot cope with large logical_block_size. like FAT, for
> example. am I wrong?

Yes indeed. When we discussed the patch adding the partial I/O, increasing the
size of logical_block was considered. The reason we didn't go the easy path was
that not all block users could handle 64k blocks. FAT is one of them.

Jerome

> 
> 	-ss
> 
--
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
Minchan Kim Jan. 10, 2017, 10:54 p.m. UTC | #3
Hi Jerome, Sergey

On Mon, Jan 09, 2017 at 10:09:20AM -0500, Jerome Marchand wrote:
> 
> 
> ----- Original Message -----
> > From: "Sergey Senozhatsky" <sergey.senozhatsky@gmail.com>
> > To: "Minchan Kim" <minchan@kernel.org>
> > Cc: "Jens Axboe" <axboe@kernel.dk>, "Hyeoncheol Lee" <cheol.lee@lge.com>, linux-block@vger.kernel.org,
> > linux-kernel@vger.kernel.org, "Andrew Morton" <akpm@linux-foundation.org>, "Sergey Senozhatsky"
> > <sergey.senozhatsky@gmail.com>, "Jerome Marchand" <jmarchan@redhat.com>
> > Sent: Monday, January 9, 2017 3:33:44 PM
> > Subject: Re: [RFC] blk: increase logical_block_size to unsigned int

Remove Robert's mail. It didn't work and don't know his update mail

> > 
> > On (01/09/17 14:04), Minchan Kim wrote:
> > > Mostly, zram is used as swap system on embedded world so it want to do IO
> > > as PAGE_SIZE aligned/size IO unit. For that, one of the problem was
> > > blk_queue_logical_block_size(zram->disk->queue, PAGE_SIZE) made overflow
> > > in *64K page system* so [1] changed it to constant 4096.
> > > Since that, partial IO can happen so zram should handle it which makes zram
> > > complicated[2].
> > > 
> > 
> > I thought that zram partial IO support is there because some file
> > systems cannot cope with large logical_block_size. like FAT, for
> > example. am I wrong?
> 
> Yes indeed. When we discussed the patch adding the partial I/O, increasing the
> size of logical_block was considered. The reason we didn't go the easy path was
> that not all block users could handle 64k blocks. FAT is one of them.


I thought it might make some FSes which doesn't support 64K block but
I didn't know what FSes exactly. I thought most popular FSes in linux
may work well(e.g., ext, btrfs, xfs). Thanks for the pointer.
I guess there might be more as well as FAT so let's keep it.

Thanks, Sergey and Jerome!


--
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 mbox

Patch

diff --git a/block/blk-settings.c b/block/blk-settings.c
index f679ae122843..0d644f37e3c6 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -349,7 +349,7 @@  EXPORT_SYMBOL(blk_queue_max_segment_size);
  *   storage device can address.  The default of 512 covers most
  *   hardware.
  **/
-void blk_queue_logical_block_size(struct request_queue *q, unsigned short size)
+void blk_queue_logical_block_size(struct request_queue *q, unsigned int size)
 {
 	q->limits.logical_block_size = size;
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c47c358ba052..0aaea317a7f4 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -281,7 +281,7 @@  struct queue_limits {
 	unsigned int		discard_granularity;
 	unsigned int		discard_alignment;
 
-	unsigned short		logical_block_size;
+	unsigned int		logical_block_size;
 	unsigned short		max_segments;
 	unsigned short		max_integrity_segments;
 
@@ -991,7 +991,7 @@  extern void blk_queue_max_discard_sectors(struct request_queue *q,
 		unsigned int max_discard_sectors);
 extern void blk_queue_max_write_same_sectors(struct request_queue *q,
 		unsigned int max_write_same_sectors);
-extern void blk_queue_logical_block_size(struct request_queue *, unsigned short);
+extern void blk_queue_logical_block_size(struct request_queue *, unsigned int);
 extern void blk_queue_physical_block_size(struct request_queue *, unsigned int);
 extern void blk_queue_alignment_offset(struct request_queue *q,
 				       unsigned int alignment);
@@ -1216,9 +1216,9 @@  static inline unsigned int queue_max_segment_size(struct request_queue *q)
 	return q->limits.max_segment_size;
 }
 
-static inline unsigned short queue_logical_block_size(struct request_queue *q)
+static inline unsigned int queue_logical_block_size(struct request_queue *q)
 {
-	int retval = 512;
+	unsigned int retval = 512;
 
 	if (q && q->limits.logical_block_size)
 		retval = q->limits.logical_block_size;
@@ -1226,7 +1226,7 @@  static inline unsigned short queue_logical_block_size(struct request_queue *q)
 	return retval;
 }
 
-static inline unsigned short bdev_logical_block_size(struct block_device *bdev)
+static inline unsigned int bdev_logical_block_size(struct block_device *bdev)
 {
 	return queue_logical_block_size(bdev_get_queue(bdev));
 }