FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 07-03-2012, 02:34 AM
Vivek Goyal
 
Default block: add sysfs entry for discard_alignment

On Mon, Jul 02, 2012 at 03:20:23PM +0200, Paolo Bonzini wrote:
> The next patches will actually use the alignment, expose it in sysfs
> for ease of debugging.
>

Don't we already have discard_alignment exported as device attribute.

/sys/block/<dev>/discard_alignment

Thanks
Vivek

> Cc: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block/blk-sysfs.c | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index aa41b47..95e919c 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -146,6 +146,11 @@ static ssize_t queue_io_opt_show(struct request_queue *q, char *page)
> return queue_var_show(queue_io_opt(q), page);
> }
>
> +static ssize_t queue_discard_alignment_show(struct request_queue *q, char *page)
> +{
> + return queue_var_show(q->limits.discard_alignment, page);
> +}
> +
> static ssize_t queue_discard_granularity_show(struct request_queue *q, char *page)
> {
> return queue_var_show(q->limits.discard_granularity, page);
> @@ -343,6 +348,11 @@ static struct queue_sysfs_entry queue_io_opt_entry = {
> .show = queue_io_opt_show,
> };
>
> +static struct queue_sysfs_entry queue_discard_alignment_entry = {
> + .attr = {.name = "discard_alignment", .mode = S_IRUGO },
> + .show = queue_discard_alignment_show,
> +};
> +
> static struct queue_sysfs_entry queue_discard_granularity_entry = {
> .attr = {.name = "discard_granularity", .mode = S_IRUGO },
> .show = queue_discard_granularity_show,
> @@ -403,6 +413,7 @@ static struct attribute *default_attrs[] = {
> &queue_io_min_entry.attr,
> &queue_io_opt_entry.attr,
> &queue_discard_granularity_entry.attr,
> + &queue_discard_alignment_entry.attr,
> &queue_discard_max_entry.attr,
> &queue_discard_zeroes_data_entry.attr,
> &queue_nonrot_entry.attr,
> --
> 1.7.1
>
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 07-03-2012, 02:00 PM
Vivek Goyal
 
Default block: add sysfs entry for discard_alignment

On Tue, Jul 03, 2012 at 01:51:13PM +0200, Paolo Bonzini wrote:
> Il 03/07/2012 04:34, Vivek Goyal ha scritto:
> >> > The next patches will actually use the alignment, expose it in sysfs
> >> > for ease of debugging.
> >> >
> > Don't we already have discard_alignment exported as device attribute.
> >
> > /sys/block/<dev>/discard_alignment
>
> Ah, interesting, I missed it completely. I guess it's because queue/
> directories only exist for full disks, and the correct alignment varies
> for each partition. So this patch is unnecessary.

Partition discard_alignments are available in
/sys/block/<disk>/<partition>/discard_alignment.

That raises an interesting question for patch3. If the discard is happening to
a partition, shouldn't you be looking at partition discard_alignment
instead of always looking at queue discard_alignment?

Thanks
Vivek

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 07-03-2012, 02:39 PM
Vivek Goyal
 
Default block: add sysfs entry for discard_alignment

On Tue, Jul 03, 2012 at 04:21:34PM +0200, Paolo Bonzini wrote:

[..]
> > That raises an interesting question for patch3. If the discard is happening to
> > a partition, shouldn't you be looking at partition discard_alignment
> > instead of always looking at queue discard_alignment?
>
> Good point! Like this?

This looks better.
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ba43f40..3530764 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1125,6 +1125,16 @@ static inline int queue_limit_discard_alignment(struct queue_limits *lim, sector
> & (lim->discard_granularity - 1);
> }
>
> +static inline int bdev_discard_alignment(struct block_device *bdev)
> +{
> + struct request_queue *q = bdev_get_queue(bdev);
> +
> + if (bdev != bdev->bd_contains)
> + return bdev->bd_part->discard_alignment;
> +
> + return q->limits.discard_alignment;
> +}
> +
> static inline unsigned int queue_discard_zeroes_data(struct request_queue *q)
> {
> if (q->limits.max_discard_sectors && q->limits.discard_zeroes_data == 1)
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index b2bde5c..77d8869 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -58,7 +58,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> /* Zero-sector (unknown) and one-sector granularities are the same. */
> granularity = max(q->limits.discard_granularity >> 9, 1U);
> mask = granularity - 1;
> - alignment = (q->limits.discard_alignment >> 9) & mask;
> + alignment = bdev_discard_alignment(bdev) >> 9;

Why are you removing AND with mask operation? I don't see any AND
operation being done in bdev_discard_alignment().

Thanks
Vivek

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 07-03-2012, 02:45 PM
Vivek Goyal
 
Default block: add sysfs entry for discard_alignment

On Tue, Jul 03, 2012 at 04:40:40PM +0200, Paolo Bonzini wrote:
> Il 03/07/2012 16:39, Vivek Goyal ha scritto:
> >> > +static inline int bdev_discard_alignment(struct block_device *bdev)
> >> > +{
> >> > + struct request_queue *q = bdev_get_queue(bdev);
> >> > +
> >> > + if (bdev != bdev->bd_contains)
> >> > + return bdev->bd_part->discard_alignment;
> >> > +
> >> > + return q->limits.discard_alignment;
> >> > +}
> >> > +
> >> > static inline unsigned int queue_discard_zeroes_data(struct request_queue *q)
> >> > {
> >> > if (q->limits.max_discard_sectors && q->limits.discard_zeroes_data == 1)
> >> > diff --git a/block/blk-lib.c b/block/blk-lib.c
> >> > index b2bde5c..77d8869 100644
> >> > --- a/block/blk-lib.c
> >> > +++ b/block/blk-lib.c
> >> > @@ -58,7 +58,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> >> > /* Zero-sector (unknown) and one-sector granularities are the same. */
> >> > granularity = max(q->limits.discard_granularity >> 9, 1U);
> >> > mask = granularity - 1;
> >> > - alignment = (q->limits.discard_alignment >> 9) & mask;
> >> > + alignment = bdev_discard_alignment(bdev) >> 9;
> > Why are you removing AND with mask operation? I don't see any AND
> > operation being done in bdev_discard_alignment().
>
> For partitions it is done by queue_limits_discard_alignment. For disks,
> it shouldn't be necessary at all but I can leave it.

Ok. I am fine with both with and without mask.

Thanks
Vivek

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 08:53 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org