linux-ext4 - Re: [PATCH 4/4] Support discard if at least one underlying device supports it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.1007021540150.6197@hs20-bc2-1.build.redhat.com>
Date:	Fri, 2 Jul 2010 15:49:49 -0400 (EDT)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Mike Snitzer <snitzer@...hat.com>
cc:	dm-devel@...hat.com, Alasdair G Kergon <agk@...hat.com>,
	linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 4/4] Support discard if at least one underlying device
 supports it

> As we discussed, we have a challenge where we need DM to avoid issuing
> a barrier before the discard IFF a target doesn't support the discard
> (which the barrier is paired with).
> 
> My understanding is that blkdev_issue_discard() only cares if the
> discard was supported.  Barrier is used just to decorate the discard
> (for correctness).  So by returning -EOPNOTSUPP we're saying the discard
> isn't supported; we're not making any claims about the implict barrier,
> so best to avoid the barrier entirely.
> 
> Otherwise we'll be issuing unnecessary barriers (and associated
> performance loss).
> 
> So yet another TODO item... Anyway:
> 
> Acked-by: Mike Snitzer <snitzer@...hat.com>

Unnecessary barriers are issued anyway. With each freed extent.

The code must issue a "SYNCHRONIZE CACHE" to flush cache for previous 
writes, then "UNMAP" and then another "SYNCHRONIZE CACHE" to commit that 
unmap to disk. And this in loop for all extents in 
"release_blocks_on_commit".

One idea behind "discard barriers" was to submit a discard request and not 
wait for it. Then the request would need a barrier so that it doesn't get 
reordered with further writes (that may potentially write to the same area 
as the discarded area). But discard isn't used this way anyway, 
sb_issue_discard waits for completion, so the barrier isn't needed.

Even if ext4 developers wanted asynchronous discard requests, they should 
fire all the discards at once and then submit one zero-sized barrier. Not 
barrier with each discard request.

This is up to ext4 developers to optimize and remove the barriers and we 
can't do anything with it. Just send "SYNCHRONIZE 
CACHE"+"UNMAP"+"SYNCHRONIZE CACHE" like the barrier specification wants...

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html