On 2020/5/14 23:42, Hillf Danton wrote:
> On Thu, 14 May 2020 13:12:18 +0800 Rong Chen wrote:
>> On 5/14/20 12:27 PM, Christian Kujau wrote:
>>> On Tue, 12 May 2020, kernel test robot wrote:
>>>> FYI, we noticed a -71.8% regression of netperf.Throughput_total_tps due to commit:
>>> As noted in this report, netperf is used to "measure various aspect of
>>> networking performance". Are we sure the bisect is correct? JFS is a
>>> filesystem and is not touching net/ in any way. So, having not attempted
>>> to reproduce this, maybe the JFS commit is a red herring?
>>>
>>> C.
>> Hi,
>>
>> The commit also causes -74.6% regression of will-it-scale.per_thread_ops:
>>
>> in testcase: will-it-scale
>> on test machine: 8 threads Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz with 16G memory
>> with following parameters:
>>
>> nr_task: 100%
>> mode: thread
>> test: unlink2
>> cpufreq_governor: performance
>> ucode: 0x21
>>
>> I'll send another report for this regression.
>>
>> Best Regards,
>> Rong Chen
> Hi
>
> Would it make sense in terms of making robot and fuzzer happy to replace
> spin lock with memory barrier, say the changes below?
>
> Hillf
>
> --- a/fs/jfs/jfs_txnmgr.c
> +++ b/fs/jfs/jfs_txnmgr.c
> @@ -416,6 +416,7 @@ tid_t txBegin(struct super_block *sb, in
> * memset(tblk, 0, sizeof(struct tblock));
> */
> tblk->next = tblk->last = tblk->xflag = tblk->flag = tblk->lsn = 0;
> + smp_wmb(); /* match mb in txLazyCommit() */
>
> tblk->sb = sb;
> ++log->logtid;
> @@ -2683,10 +2684,16 @@ static void txLazyCommit(struct tblock *
> {
> struct jfs_log *log;
>
> - while (((tblk->flag & tblkGC_READY) == 0) &&
> - ((tblk->flag & tblkGC_UNLOCKED) == 0)) {
> - /* We must have gotten ahead of the user thread
> - */
> + for (;;) {
> + u16 flag = tblk->flag;
> +
> + smp_rmb(); /* match mb in txBegin() */
> + if (flag & tblkGC_READY)
> + break;
> + if (flag & tblkGC_UNLOCKED)
> + break;
> +
> + /* We must have gotten ahead of the user thread */
> jfs_info("jfs_lazycommit: tblk 0x%p not unlocked", tblk);
> yield();
> }
> @@ -2698,9 +2705,9 @@ static void txLazyCommit(struct tblock *
> log = (struct jfs_log *) JFS_SBI(tblk->sb)->log;
>
> spin_lock_irq(&log->gclock); // LOGGC_LOCK
> -
> + smp_mb__after_spinlock();
> tblk->flag |= tblkGC_COMMITTED;
> -
> + smp_wmb();
> if (tblk->flag & tblkGC_READY)
> log->gcrtc--;
>
>
I think this patch is okay.
Thanks a lot, Hillf :)
Best wishes,
Jia-Ju Bai