On Tue, Mar 26, 2019 at 02:39:45PM +0200, Nikolay Borisov wrote: > On 26.03.19 г. 14:35 ч., Filipe Manana wrote: > > On Tue, Mar 26, 2019 at 12:17 PM Nikolay Borisov <nborisov@xxxxxxxx> wrote: > >> On 26.03.19 г. 12:49 ч., fdmanana@xxxxxxxxxx wrote: > >>> From: Filipe Manana <fdmanana@xxxxxxxx> > >>> > >>> Whan a filesystem is mounted with the nologreplay mount option, which > >>> requires it to be mounted in RO mode as well, we can not allow discard on > >>> free space inside block groups, because log trees refer to extents that > >>> are not pinned in a block group's free space cache (pinning the extents is > >>> precisely the first phase of replaying a log tree). > >>> > >>> So do not allow the fitrim ioctl to do anything when the filesystem is > >>> mounted with the nologreplay option, because later it can be mounted RW > >>> without that option, which causes log replay to happen and result in > >>> either a failure to replay the log trees (leading to a mount failure), a > >>> crash or some silent corruption. > >>> > >>> Reported-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > >>> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx> > >> > >> Does it make sense to make the check a bit more specific and only return > >> EROFS when NOLOGREPLAY and the log tree has non-null generation? > > > > It would make sense checking if there's actually a log tree as well. > > Neither the xfs nor ext4 (which is already in Linus' tree) do such > > equivalent checks, nor the proposed fstests test case makes sure a > > journal/log exists. > > > > Not against it, but this isn't a common use case either. > > I think of this as sorts of "optimisation" where if we don't have a tree > then we can allow trim. Though this is much simpler so I'm fine with it > as well. Agreed, the simple solution sounds ok to me, trim is not a critical operation so we don't need to try harder to make it work even with the mount option.
