Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> 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