Re: [PATCH] bcache: stop using the deprecated get_seconds()

[Coly Li]

On 2018/6/21 12:20 AM, Arnd Bergmann wrote:
> On Wed, Jun 20, 2018 at 5:51 PM, Coly Li <colyli@xxxxxxx> wrote:
>> On 2018/6/20 5:51 PM, Arnd Bergmann wrote:
>>> bcache uses get_seconds() to read the current system time and store it in
>>> the superblock as well as in uuid_entry structures that are user visible.
>>>
>>> This changes over from the deprecated function to
>>> ktime_get_real_seconds(), which returns a 64-bit timestamp as it
>>> should. Unfortunately, the two structures are still limited to 32 bits,
>>> so this won't fix any real problems. Let's at least document that
>>> properly, in case we get an updated format in the future it can be
>>> fixed. Until then, we still have some time, and checking the tools
>>> at https://github.com/koverstreet/bcache-tools reveals no access to
>>> any of them.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
>>
>> Hi Arnd,
>>
>> Firstly thanks to your patch, especially the detailed information in
>> patch log, it helps me to understand the problem more easier.
>>
>> From the information, it seems the problem is current 32bit time stamp
>> will be overflow in 2106. So it will be 88 years later, which I have to
>> say I don't care.
>>
>> Also for get_seconds() which works well for current code as many other
>> places call it, I would like to keep it.
> 
> I'm currently in the process of removing all instances of get_seconds()
> with patches like this. In many cases, we actually want to use
> ktime_get_seconds() to return a monotonic time that is immune
> to concurrent setttimeofday() calls, in others the code needs to be
> changed to avoid the y2038 overflow. For bcache, we don't
> really need either of them, but I'd still want to move over everything
> to ktime_get_* based interfaces.
> 

Hi Arnd,

Oh I see. Now I agree with you, and no more concern. Thanks for your
explaining.

> Should I clarify that motivation in the changelog text further?
> 

Yes please, that will be great.

> I can also do a simple replacement of get_seconds() with
> ktime_get_real_seconds() throughout bcache instead of
> adding the intermediate helper function.

Yes please, it will be better IMHO.

Thanks.

Coly Li