Rui Hua-- Thanks for the clarification. You're correct, it doesn't get called, and it goes to read_complete. However, read_error should probably get called. How would you suggest handling this-- should we initially set read_dirty_data true and clear it if it is all clean? (and change the condition to properly go into the error path). As it is, the patch makes things no worse but the error path is very unsafe. Mike On Thu, Nov 16, 2017 at 8:25 PM, Rui Hua <huarui.dev@xxxxxxxxx> wrote: > Hi, Michael-- > > Thanks for the quickly reply. > > If the metadata on the cache device was NOT correctly read, > cached_dev_read_error() will not be called, so the recovery check will > not be executed. so this patch is safe. > s->iop.error was not set when faild to read metadata on the cache > device, so the cached_dev_bio_complete() will be called instead of > cached_dev_read_error() > > 2017-11-17 12:02 GMT+08:00 Michael Lyle <mlyle@xxxxxxxx>: >> Hi, Rui Hua-- >> >> On 11/16/2017 07:51 PM, Rui Hua wrote: >>> In this patch, we use s->read_dirty_data to judge whether the read >>> request hit dirty data in cache device, it is safe to reread data from >>> the backing device when the read request hit clean data. This can not >>> only handle cache read race, but also recover data when failed read >>> request from cache device. >> >> I haven't read over all of this yet, to understand the read race. But >> this change can't be safe-- read_dirty_data only is set to true if the >> metadata on the cache device is correctly read. If that fails, the flag >> may not be set and we could read stale data on the backing device. >> >> Mike
