svn commit: r254138 - in head: share/man/man9 sys/amd64/amd64 sys/arm/arm sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/dev/agp sys/dev/drm2/i915 sys/dev/drm2/ttm sys/dev/md sys/fs/fuse sys/fs...

Justin T. Gibbs gibbs at FreeBSD.org
Tue Aug 20 22:27:07 UTC 2013


On Aug 13, 2013, at 8:59 AM, Attilio Rao <attilio at freebsd.org> wrote:

> On Tue, Aug 13, 2013 at 4:22 PM, Ulrich Spörlein <uqs at freebsd.org> wrote:
>> On Fri, 2013-08-09 at 11:11:12 +0000, Attilio Rao wrote:
>>> Author: attilio
>>> Date: Fri Aug  9 11:11:11 2013
>>> New Revision: 254138
>>> URL: http://svnweb.freebsd.org/changeset/base/254138
>>> 
>>> Log:
>>>  The soft and hard busy mechanism rely on the vm object lock to work.
>>>  Unify the 2 concept into a real, minimal, sxlock where the shared
>>>  acquisition represent the soft busy and the exclusive acquisition
>>>  represent the hard busy.
>>>  The old VPO_WANTED mechanism becames the hard-path for this new lock
>>>  and it becomes per-page rather than per-object.
>>>  The vm_object lock becames an interlock for this functionality:
>>>  it can be held in both read or write mode.
>>>  However, if the vm_object lock is held in read mode while acquiring
>>>  or releasing the busy state, the thread owner cannot make any
>>>  assumption on the busy state unless it is also busying it.
>>> 
>>>  Also:
>>>  - Add a new flag to directly shared busy pages while vm_page_alloc
>>>    and vm_page_grab are being executed.  This will be very helpful
>>>    once these functions happen under a read object lock.
>>>  - Move the swapping sleep into its own per-object flag
>>> 
>>>  The KPI is heavilly changed this is why the version is bumped.
>>>  It is very likely that some VM ports users will need to change
>>>  their own code.
>>> 
>>>  Sponsored by:       EMC / Isilon storage division
>>>  Discussed with:     alc
>>>  Reviewed by:        jeff, kib
>>>  Tested by:  gavin, bapt (older version)
>>>  Tested by:  pho, scottl
>> 
>> The changes to sys/vm/vm_fault.c introduce a call to
>> vm_page_sleep_if_busy() where the return code is not checked. The other
>> 5 places in the tree check the return code, please fix this here too.
>> It's CID 1062398, and I would encourage folks to get an account with
>> scan.coverity.com and have an eye on newly found defects.
> 
> Not true.
> The same call existed also before with exactly the same semantic.
> The trick there is that it is not important to check for the return
> value because we are going to retry the operation anyway.
> The code looks ok to me.
> 
> Attilio

If this is the case, perhaps the call should be prefixed with "(void)".
That would at least tell the next person reading the code that this
is intentional.

--
Justin


More information about the svn-src-head mailing list