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 -- To unsubscribe from this list: send the line "unsubscribe linux-bcache" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
