On Sunday, 30 October 2016 19:06:57 CET Theo de Raadt wrote:
> > > From: David Gwynne <da...@gwynne.id.au>

> > > whats the use of the ioctl though? why do we need extra sync cache
> > > things?
> > 
> > Right.  This smells like papering over real bugs.

If there is a power loss or a usb disk device is removed, partitions that are, 
at that point of time, not mounted or are only mounted read-only should not 
suffer data loss. The only way to ensure this is flushing the disk cache when a 
r/w mount is unmounted or changed to r/o .

The most common situation where this is an issue are devices like home 
routers, which have their root fs on a ramdisk and only store configuration 
data on the disk. When the configuration is changed, the r/o mount is changed 
to r/w, the data is written, and then the mount is changed to r/o again.

> 
> Here's the thoughts I have exchanged with sf.
> 
> This issue should not be per filesystem, not even per partition.  It
> seems this should keep track of when any partition converts from RW ->
> R access (either via direct open, or via mount), and if so schedule
> the required sync operation (to occur sometime soon).  That way if
> many such RW->R transitions occur, they won't serialize and create a
> performance hit.

It was also pointed out by Theo that it would be nicer if we could have the 
code in one central place instead of having the cache flush in every file 
system.

Another thing that occurred to me during the discussion is that maybe we want 
to ensure that no more than one cache flush operation is in progress on a 
single device at any time. Having several cache flushes sent to the device in 
parallel may uncover interesting firmware bugs that we don't want to deal with.

Apart from that, I am not sure that the performance hit is noticeable. The 
only case where lots of file systems are unmounted or re-mounted r/o is at 
shutdown and reboot. And I don't think that even a dozen cache flushes will 
cause more than 0.1s delay if there is not much data to flush. Also, I don't 
think that it is enough if the cache flush is scheduled to occur soon. It must 
occur before the mount/umount system call returns to userland. In this case, I 
greatly prefer correctness over performance. 

So, I think that any solution to merge several cache flush operations will add 
quite a bit of complexity, is difficult to get right, and has little gain. 
Unless I am missing a use case where the performance impact is not negligible.

> Furthermore I don't believe userland should have access to this.
> We've made it 30 years without needing this, why do we need it now.

I don't see a case where userland needs access to this, but doing this as an 
ioctl is the simplest implementation and preventing userland from using it may 
be unneeded complexity.

Cheers,
Stefan

Reply via email to