The next patches will actually use the alignment, expose it in sysfs
for ease of debugging.

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

On Mon, Jul 02 2012 at 10:34pm -0400,
Vivek Goyal <vgoyal@redhat.com> wrote:

> 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

Yes we do. It is documented in Documentation/ABI/testing/sysfs-block

So this patch isn't needed.

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

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.

Paolo

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

Il 03/07/2012 16:00, Vivek Goyal ha scritto:
>> >
>> > 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.

Yes. If it were in queue/, it wouldn't be available.

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

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;

/*
* Ensure that max_discard_sectors is of the proper

Paolo

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

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.

paolo

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