Re: [PATCH] bcache: fix race in setting bdev state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tang---

On 10/09/2017 11:51 PM, tang.junhui@xxxxxxxxxx wrote:
[snip]
> Yes, It's true. but it's hard to resolve it by holding writeback lock, 
> since not all codes in bch_writeback_thread() are held in writeback lock, 
> actually in a previous patch I have resolved it by changing schedule() to 
> schedule_timeout(WRITE_BACK_WAIT_CYCLE) to avoid permanent sleep.

I'm not sure I understand.  The deeper stuff in bch_writeback_thread()
seems deterministic and guaranteed to wake on its own-- fixed delays
during writeback and waiting for IOs to complete.  Probably the worst
thing is dc->writeback_delay is very long.

It seems like that set_current_state(TASK_INTERRUPTIBLE) is the main
defective one-- and it can be fixed by moving it to above the unlock,
and having callers of bch_writeback_queue first acquire a shared
writeback lock.

>> There's a pretty intricate set of dependencies between the dirty bit, 
>> the state in the superblock, the refcount, the writeback lock, and the 
>> runnable state of the writeback thread.  It feels dangerous, but maybe 
>> after I draw graphs of all the dependencies I'll feel better.
> 
>> Writeback locking needs refactoring for performance anyways-- maybe this 
>> can simplified at the same time.
> Yes, we'd better make clear the process, and find the issues and point to 
> do refactor, but I think it is a long run, we need to do it step by step.

Yes, we should come up with a plan.

I like traversing the writeback lock on detach because all the critical
paths that touch state that detach cares about-- superblock state, dirty
state--- happen with the writeback lock acquired.  So it's easier to
reason about what happens if we acquire the lock exclusively-- we can be
sure that no one will set the device dirty again, that the wakeup to
writeback happens, that the ways that refcount can increase are limited,
etc.

That is, there's a lot less things to think about at once to be sure
it's correct.

Also, it seems like we need to do this to wake up the writeback thread
properly (rather than making it wake up and poll for detach).

> 
> Regards,
> Tang

Mike




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux