On Thursday 31 July 2014 at 23:29:05, Edward Shishkin wrote:
>
> On 07/29/2014 11:29 AM, Ivan Shapovalov wrote:
> > On Monday 28 July 2014 at 13:46:01, Edward Shishkin wrote:
> >> Thanks, looks OK (I haven't tested it yet though..)
> >>
> >> I suggest to release a version without garbage collection for now
> >> (just pass the sorted and merged extents to blkdev_issue_discard).
> > Yeah, seems reasonable.
> > I'll submit v7 of "initial implementation" shortly with simplified discard_extent(),
> > then we could apply two patchsets and that'll be it.
> >
> > ----
> > Regarding the "ideal" solution
>
>
> BTW, do we need this "ideal solution" at all?
> For example, my SSD Samsung 840 EVO shows erase_unit = 512 bytes,
> and alignment = 0. It means that in my case everything will be discarded
> in the best form with the simplified discard_extent().
>
> I am not lazy, just want to make sure we do useful work :)
> May be it makes sense to perform a small investigation of the SSD market
> first?
Erase unit can't be 512 bytes. Well, it can, but this is _highly_ unlikely...
(byte < page < block; writes are per-page, erases are per-block, and
minimal common page size is 512 bytes)
Try to check with `hdparm -I /dev/sdX | grep TRIM`, or, even better, pick an
unused sector, write something to it and try to discard it using
`hdparm --trim-sector-ranges`, then check the sector's contents.
>
>
> > ----
> >
> > It seems that reiser4_alloc_blocks() can't be told to "allocate this exact
> > count of blocks or fail". It allocates any number of blocks from 1 to needed,
> > and actual count is returned in *len.
> >
> > Is it better to add an "exact" flag to reiser4_blocknr_hint or to add an
> > "allocate" flag to reiser4_check_blocks()?
>
>
> Let's decide what we want from check_blocks().
>
> When (discard_offset % block_size != 0) we'll always need (even in the
> case of empty headp (tailp) to check the left (right) neighbor block of
> the extent. If it is busy, then we'll need to reduce alen (alen -= d_uni)
> because of the "shift".
>
> It means that we want check_blocks() to do the following:
> . go leftward (rightward) and allocate not more than @count continuous
> free blocks;
> . return actual number of allocated blocks.
>
> It seems that currently we don't have suitable primitives in our space
> allocator. I prefer to not modify existing ones and write a new primitive.
Yeah, after another glance through bitmap code this seems to be the cleanest
way. The alloc_blocks_{forward,backward}_bitmap() functions simply convert the
hint to an "internal representation" (search start/end, length min/max) and
call bitmap_alloc_{forward,backward}().
I'd write something like alloc_blocks_*_exact_bitmap(start, min_len, max_len)
eventually calling into the same bitmap_alloc_*() functions. They also can be
reused in the batch discard code (if we decide to implement it). OK?
>
> Edward.
Thanks,
--
Ivan Shapovalov / intelfx /Attachment:
signature.asc
Description: This is a digitally signed message part.
