Query about dm device max_discard_sectors

Query about dm device max_discard_sectors

[Amar Mudrankit]

[-- Attachment #1.1: Type: text/plain, Size: 1485 bytes --]

Hi all,

I am building an experimental device mapper target which is supposed to
receive
all TRIM requests (REQ_DISCARD).  Irrespective of whether underlying actual
physical block device supports discards or not, my device mapper target
should
receive REQ_DISCARD bios.

In my target, I set:

ti->num_discard_requests = 1;
ti->discards_supported = 1;

It looks like setting above 2 parameters did not get me any REQ_DISCARD
requests when "fstrim" utility is used.  The operations fails with
"Operation not
supported" from kernel funcion:

blkdev_issue_discard

because max_discard_sectors for device queue is set to 0.

max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
if (unlikely(!max_discard_sectors))
       return -EOPNOTSUPP;

When I further digged into the device mapper code, I found that
dm_calculate_queue_limits function sets the queue limits to default values
of 0 for both limits as well as ti_limits and thus blk_stack_limits
eventually
sets max_discard_sectors to 0 for the device mapper target.

So, if my device mapper target does not implement iterate_devices function
(used to set max_discard_sectors based on underlying physical device's
discard
support) or underlying physical device does not support discards, then there
is no way for device mapper target to set max_discard_sectors more than 0
to receive REQ_DISCARD commands.

Is this analysis correct? Or I am missing something?

Appreciate the help / advice.

Thanks and Regards,
Amar

[-- Attachment #1.2: Type: text/html, Size: 1675 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Query about dm device max_discard_sectors

[Mike Snitzer]

On Sun, Mar 25 2012 at  6:46pm -0400,
Amar Mudrankit <amar.mudrankit@gmail.com> wrote:

> Hi all,
> 
> I am building an experimental device mapper target which is supposed to
> receive
> all TRIM requests (REQ_DISCARD).  Irrespective of whether underlying actual
> physical block device supports discards or not, my device mapper target
> should
> receive REQ_DISCARD bios.

I'm just curious but: what will your target do in general?
 
> In my target, I set:
> 
> ti->num_discard_requests = 1;
> ti->discards_supported = 1;

You'll also want to set ti->discards_supported

see: http://git.kernel.org/linus/4c259327

> It looks like setting above 2 parameters did not get me any REQ_DISCARD
> requests when "fstrim" utility is used.  The operations fails with
> "Operation not
> supported" from kernel funcion:
> 
> blkdev_issue_discard
> 
> because max_discard_sectors for device queue is set to 0.
> 
> max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
> if (unlikely(!max_discard_sectors))
>        return -EOPNOTSUPP;
> 
> When I further digged into the device mapper code, I found that
> dm_calculate_queue_limits function sets the queue limits to default values
> of 0 for both limits as well as ti_limits and thus blk_stack_limits
> eventually
> sets max_discard_sectors to 0 for the device mapper target.
> 
> So, if my device mapper target does not implement iterate_devices function
> (used to set max_discard_sectors based on underlying physical device's
> discard
> support) or underlying physical device does not support discards, then there
> is no way for device mapper target to set max_discard_sectors more than 0
> to receive REQ_DISCARD commands.
> 
> Is this analysis correct? Or I am missing something?

All DM targets should implement .iterate_devices -- intent is to iterate
over all data devices.

You'll also want to implement .io_hints to set limits->max_discard_sectors 
(like drivers/md/dm-thin.c does).

Mike

Re: Query about dm device max_discard_sectors

[Mike Snitzer]

On Sun, Mar 25 2012 at  7:54pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Sun, Mar 25 2012 at  6:46pm -0400,
> Amar Mudrankit <amar.mudrankit@gmail.com> wrote:
> 
> > Hi all,
> > 
> > I am building an experimental device mapper target which is supposed to
> > receive
> > all TRIM requests (REQ_DISCARD).  Irrespective of whether underlying actual
> > physical block device supports discards or not, my device mapper target
> > should
> > receive REQ_DISCARD bios.
> 
> I'm just curious but: what will your target do in general?
>  
> > In my target, I set:
> > 
> > ti->num_discard_requests = 1;
> > ti->discards_supported = 1;
> 
> You'll also want to set ti->discards_supported
> 
> see: http://git.kernel.org/linus/4c259327

ah, I now see you already did set discards_supported.
 
> > So, if my device mapper target does not implement iterate_devices function
> > (used to set max_discard_sectors based on underlying physical device's
> > discard
> > support) or underlying physical device does not support discards, then there
> > is no way for device mapper target to set max_discard_sectors more than 0
> > to receive REQ_DISCARD commands.
> > 
> > Is this analysis correct? Or I am missing something?
> 
> All DM targets should implement .iterate_devices -- intent is to iterate
> over all data devices.
> 
> You'll also want to implement .io_hints to set limits->max_discard_sectors 
> (like drivers/md/dm-thin.c does).

BTW, the dm-thin.c discard changes will be merged in 3.4, you can see
the relevant patches here:

http://people.redhat.com/agk/patches/linux/editing/dm-thin-support-discards.patch
http://people.redhat.com/agk/patches/linux/editing/dm_thin-add-pool-target-flags-to-control-discard.patch

Re: Query about dm device max_discard_sectors

[Amar Mudrankit]

[-- Attachment #1.1: Type: text/plain, Size: 2421 bytes --]

On Sun, Mar 25, 2012 at 9:49 PM, Mike Snitzer <snitzer@redhat.com> wrote:

> On Sun, Mar 25 2012 at  7:54pm -0400,
> Mike Snitzer <snitzer@redhat.com> wrote:
>
> > On Sun, Mar 25 2012 at  6:46pm -0400,
> > Amar Mudrankit <amar.mudrankit@gmail.com> wrote:
> >
> > > Hi all,
> > >
> > > I am building an experimental device mapper target which is supposed to
> > > receive
> > > all TRIM requests (REQ_DISCARD).  Irrespective of whether underlying
> actual
> > > physical block device supports discards or not, my device mapper target
> > > should
> > > receive REQ_DISCARD bios.
> >
> > I'm just curious but: what will your target do in general?
>

Target is somewhat similar to what a Flash Translation Layer does in SSDs,
but not
as fancy as that.  So, when my target maintains logical -> physical
mapping, my target
will be interested in which blocks are free from file system perspective, a
typical use
case of TRIM.  I guess somewhat similar to thin-provisioning.

>
> > > In my target, I set:
> > >
> > > ti->num_discard_requests = 1;
> > > ti->discards_supported = 1;
> >
> > You'll also want to set ti->discards_supported
> >
> > see: http://git.kernel.org/linus/4c259327
>
> ah, I now see you already did set discards_supported.
>
> > > So, if my device mapper target does not implement iterate_devices
> function
> > > (used to set max_discard_sectors based on underlying physical device's
> > > discard
> > > support) or underlying physical device does not support discards, then
> there
> > > is no way for device mapper target to set max_discard_sectors more
> than 0
> > > to receive REQ_DISCARD commands.
> > >
> > > Is this analysis correct? Or I am missing something?
> >
> > All DM targets should implement .iterate_devices -- intent is to iterate
> > over all data devices.
> >
> > You'll also want to implement .io_hints to set
> limits->max_discard_sectors
> > (like drivers/md/dm-thin.c does).
>
> BTW, the dm-thin.c discard changes will be merged in 3.4, you can see
> the relevant patches here:
>
>
> http://people.redhat.com/agk/patches/linux/editing/dm-thin-support-discards.patch
>
> http://people.redhat.com/agk/patches/linux/editing/dm_thin-add-pool-target-flags-to-control-discard.patch
>

With io_hints function pointer, I set max_discard_sectors and it is getting
set properly,  and I
was able to receive and process TRIM requests.

Thank you Mike for the help !!

Thanks and Regards,
Amar

[-- Attachment #1.2: Type: text/html, Size: 3711 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]