On Aug 17, 2017, at 3:21 AM, Jan Kara <jack@xxxxxxx> wrote:
>
> On Thu 17-08-17 11:19:59, Jan Kara wrote:
>> Hi Shilong!
>>
>> On Thu 17-08-17 06:23:26, Wang Shilong wrote:
>>> thanks for good suggestion, just one question we could not hold lock
>>> with nojounal mode, how about something attached one?
>>>
>>> please let me know if you have better taste for it, much appreciated!
>>
>> Thanks for quickly updating the patch! Is the only reason why you cannot
>> hold the lock in the nojournal mode that sb_getblk() might sleep? The
>> attached patch should fix that so that you don't have to special-case the
>> nojournal mode anymore.
>
> Forgot to attach the patch - here it is. Feel free to include it in your
> series as a preparatory patch.
Strange, I never even knew recently_deleted() existed, even though it was
added to the tree 4 years ago yesterday. It looks like this is only used
with the no-journal code, which I don't really interact with.
One thing I did notice when looking at it is that there is a Y2038 bug in
recently_deleted(), as it is comparing 32-bit i_dtime directly with 64-bit
get_seconds(). To fix this, it would be possible to either use a wrapped
32-bit comparison, like time_after() for jiffies, something like:
u32 now, dtime;
/* assume dtime is within the past 30 years, see time_after() */
now = get_seconds();
if (dtime && (dtime - now < 0) && (dtime + recentcy - now < 0))
ret = 1;
or use i_ctime_extra to implicitly extend i_dtime beyond 2038, something like:
/* assume dtime epoch same as ctime, see EXT4_INODE_GET_XTIME() */
dtime = le32_to_cpu(raw_inode->i_dtime);
if (EXT4_INODE_SIZE(sb) > EXT4_GOOD_OLD_INODE_SIZE &&
offsetof(typeof(*raw_inode), i_ctime_extra) + 4 <=
EXT4_GOOD_OLD_INODE_SIZE + le32_to_cpu(raw_inode->i_extra_isize))
dtime += (long)(le32_to_cpu(raw_inode->i_ctime_extra) &
EXT4_EPOCH_MASK) << 32;
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
