Re: CVS commit: src/sys/uvm

[yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi)]

[ moving to tech-kern ]

hi,

> yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi) wrote:
>> > 
>> > Here is the updated patch after your changes:
>> > 
>> > http://www.netbsd.org/~rmind/uvm_anon_freelst2.diff
>> > 
>> > As you noted, uvm_anfree() can temporarily release the amap lock - that
>> > can happen in amap_copy().  Patch closes the race by moving uvm_anfree
>> > () further, and changes the semantics of the function, now called
>> > uvm_anon_freelst(), to return with amap lock released (plus free anons
>> > without lock held).
>> 
>> the temporary release of the amap lock is only for O->A loan
>> which you disabled, isn't it?
> 
> Right, uvm_anon_locklaonpg() dance can happen only in O->A case.  However,
> having uvm_anfree() able to release the lock by its interface definition
> is potentially defective.  It is the main motivation why I want to slightly
> rework the code into uvm_anon_freelst() which would always drop the lock
> and move freeing of anons to the end point.  Cleaner, less error prone.

the committed change seems broken in case uvm_anon_dispose sets PG_RELEASED.
in that case, uvm_anon_freelst should leave the anon as it will be freed by
uvm_anon_release later.

YAMAMOTO Takashi

> 
> -- 
> Mindaugas