Hi, Michael Thanks for your reminder. I'll checkpatch carefully next time. Thanks 2017-11-22 13:13 GMT+08:00 Michael Lyle <mlyle@xxxxxxxx>: > Reviewed-by: Michael Lyle <mlyle@xxxxxxxx> > > Please note though, that your patches have all tabs expanded to spaces, > which makes them difficult to apply. I have fixed things these times > but please try to submit them in correct form in the future. > > Thanks, > > Mike > > On 11/17/2017 04:57 PM, Michael Lyle wrote: >> 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 >> -- >> 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 >> >
