On Fri, Dec 2, 2011 at 5:56 PM, Michal Hocko <mhocko@xxxxxxx> wrote:
> On Fri 02-12-11 17:42:11, Bob Liu wrote:
>> There are multi places need to get swap_cgroup, so add a helper
>> function:
>> static struct swap_cgroup *swap_cgroup_getsc(swp_entry_t ent);
>> to simple the code.
>
> I like the cleanup but I guess we can do a little bit better ;)
>
> [...]
>> +static struct swap_cgroup *swap_cgroup_getsc(swp_entry_t ent)
>
> Add struct swap_cgroup_ctrl ** ctrl parameter
>
>> +{
>> + int type = swp_type(ent);
>> + unsigned long offset = swp_offset(ent);
>> + unsigned long idx = offset / SC_PER_PAGE;
>> + unsigned long pos = offset & SC_POS_MASK;
>> + struct swap_cgroup_ctrl *ctrl;
>> + struct page *mappage;
>> + struct swap_cgroup *sc;
>> +
>> + ctrl = &swap_cgroup_ctrl[type];
>
> if (ctrl)
> *ctrl = &swap_cgroup_ctrl[type]
>
> [...]
>> @@ -375,20 +393,14 @@ unsigned short swap_cgroup_cmpxchg(swp_entry_t ent,
>> unsigned short old, unsigned short new)
>> {
>> int type = swp_type(ent);
>> - unsigned long offset = swp_offset(ent);
>> - unsigned long idx = offset / SC_PER_PAGE;
>> - unsigned long pos = offset & SC_POS_MASK;
>> struct swap_cgroup_ctrl *ctrl;
>> - struct page *mappage;
>> struct swap_cgroup *sc;
>> unsigned long flags;
>> unsigned short retval;
>>
>> ctrl = &swap_cgroup_ctrl[type];
>> + sc = swap_cgroup_getsc(ent);
>
> sc = swap_cgroup_getsc(ent, &ctrl);
> [...]
>> @@ -410,20 +422,14 @@ unsigned short swap_cgroup_cmpxchg(swp_entry_t ent,
>> unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
>> {
>> int type = swp_type(ent);
>> - unsigned long offset = swp_offset(ent);
>> - unsigned long idx = offset / SC_PER_PAGE;
>> - unsigned long pos = offset & SC_POS_MASK;
>> struct swap_cgroup_ctrl *ctrl;
>> - struct page *mappage;
>> struct swap_cgroup *sc;
>> unsigned short old;
>> unsigned long flags;
>>
>> ctrl = &swap_cgroup_ctrl[type];
>> + sc = swap_cgroup_getsc(ent);
>
> Same here
>
> [...]
>> @@ -440,21 +446,10 @@ unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
>> */
>> unsigned short lookup_swap_cgroup(swp_entry_t ent)
>> {
>> - int type = swp_type(ent);
>> - unsigned long offset = swp_offset(ent);
>> - unsigned long idx = offset / SC_PER_PAGE;
>> - unsigned long pos = offset & SC_POS_MASK;
>> - struct swap_cgroup_ctrl *ctrl;
>> - struct page *mappage;
>> struct swap_cgroup *sc;
>> - unsigned short ret;
>>
>> - ctrl = &swap_cgroup_ctrl[type];
>> - mappage = ctrl->map[idx];
>> - sc = page_address(mappage);
>> - sc += pos;
>> - ret = sc->id;
>> - return ret;
>> + sc = swap_cgroup_getsc(ent);
>> + return sc->id;
>
> return swap_cgroup_getsc(ent, NULL)->id;
>
> What do you think?
Alright, i'll send out v2.
Thanks for your review.
--
Regards,
--Bob
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href