On Thu, 21 Jul 2005, Nigel Cunningham wrote:
> This patch implements a new PF_SYNCTHREAD flag, which allows upcoming
> the refrigerator implementation to know that a thread is syncing data to
> disk. This allows the refrigerator to be more reliable, even under heavy
> load, because it can separate userspace processes that are submitting
> I/O from those trying to sync it and freezing the former first. We can
> thus ensure that syncing processes complete their work more quickly and
> the refrigerator is far less likely to incorrectly give up (thinking a
> syncing process is completely failing to enter the refrigerator).
I don't have any strong feelings for this, one way or another. It seems
kinda hacky, and it needs to be discussed publically, and it seems like it
definitely seems like it doesn't need to go in immediately.
I have just a couple of suggestions below..
> int fsync_super(struct super_block *sb)
> {
> + int ret;
> +
> + /* A safety net. During suspend, we might overwrite
> + * memory containing filesystem info. We don't then
> + * want to sync it to disk. */
> + BUG_ON(test_suspend_state(SUSPEND_DISABLE_SYNCING));
Please do not add any new BUG()s. Figure out another way to gracefully
fail, perhaps some place else.
> + current->flags |= PF_SYNCTHREAD;
Is all the modification of current->flags safe? It seems like you could be
pre-empted in the middle and things could get wacky.
Another note is that these changes will have to go through Al Viro, who
might have some suggestions on the Right(tm) way to do it.
Pat
