Messages in this thread | | | From | "Huang\, Ying" <> | Subject | Re: [PATCH 1/6] swap: Add comments to lock_cluster_or_swap_info() | Date | Sat, 14 Jul 2018 12:07:43 +0800 |
| |
Dave Hansen <dave.hansen@linux.intel.com> writes:
>> +/* >> + * At most times, fine grained cluster lock is sufficient to protect > > Can we call out those times, please?
To protect si->swap_map[], if HDD, si->lock is used, otherwise cluster lock is used. "at most times" is ambiguous here, I will fix it.
>> + * the operations on sis->swap_map. > > Please be careful with the naming. You can call it 'si' because that's > what the function argument is named. Or, swap_info_struct because > that's the struct name. Calling it 'sis' is a bit sloppy, no? > >> No need to acquire gross grained > > "coarse" is a conventional antonym for "fine".
Sorry for my poor English, will change this.
>> + * sis->lock. But cluster and cluster lock isn't available for HDD, >> + * so sis->lock will be instead for them. >> + */ >> static inline struct swap_cluster_info *lock_cluster_or_swap_info( >> struct swap_info_struct *si, >> unsigned long offset) > > What I already knew was: there are two locks. We use one sometimes and > the other at other times. > > What I don't know is why there are two locks, and the heuristics why we > choose between them. This comment doesn't help explain the things I > don't know.
cluster lock is used to protect fields of struct swap_cluster_info, and si->swap_map[], this is described in comments of struct swap_cluster_info. si->lock is used to protect other fields of si. If two locks need to be held, hold si->lock first. This is for non-HDD. For HDD, there are no cluster, so si->lock is used to protect si->swap_map[].
Best Regards, Huang, Ying
| |