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 >
