On 10/19/15 17:01, Konstantin Belousov wrote:
On Mon, Oct 19, 2015 at 10:56:32AM +0000, Hans Petter Selasky wrote:
Author: hselasky
Date: Mon Oct 19 10:56:32 2015
New Revision: 289565
URL: https://svnweb.freebsd.org/changeset/base/289565

Log:
   Merge LinuxKPI changes from DragonflyBSD:
   - Implement pagefault_disable() and pagefault_enable().

   Sponsored by:        Mellanox Technologies

Modified:
   head/sys/ofed/include/linux/uaccess.h

Modified: head/sys/ofed/include/linux/uaccess.h
==============================================================================
--- head/sys/ofed/include/linux/uaccess.h       Mon Oct 19 10:54:24 2015        
(r289564)
+++ head/sys/ofed/include/linux/uaccess.h       Mon Oct 19 10:56:32 2015        
(r289565)
@@ -2,7 +2,8 @@
   * Copyright (c) 2010 Isilon Systems, Inc.
   * Copyright (c) 2010 iX Systems, Inc.
   * Copyright (c) 2010 Panasas, Inc.
- * Copyright (c) 2013, 2014 Mellanox Technologies, Ltd.
+ * Copyright (c) 2013-2015 Mellanox Technologies, Ltd.
+ * Copyright (c) 2015 Fran??ois Tigeot
   * All rights reserved.
   *
   * Redistribution and use in source and binary forms, with or without
@@ -32,4 +33,14 @@
  #define       get_user(_x, _p)        -copyin((_p), &(_x), sizeof(*(_p)))
  #define       put_user(_x, _p)        -copyout(&(_x), (_p), sizeof(*(_p)))

+static inline void pagefault_disable(void)
+{
+       curthread_pflags_set(TDP_NOFAULTING | TDP_RESETSPUR);
+}
+
+static inline void pagefault_enable(void)
+{
+       curthread_pflags_restore(~(TDP_NOFAULTING | TDP_RESETSPUR));
+}
+
  #endif        /* _LINUX_UACCESS_H_ */

This is wrong on many counts.  First, we already provide the wrappers to
implement the nofaulting behaviour, see vm_fault_disable_pagefaults()
and vm_fault_enable_pagefaults().

Second, your implementation ignores possible recursion on the
state, unconditionally re-enabling faulting, and more seriously, it
unconditionally obliterates the spurious faults tracker.

The return value from vm_fault_disable_pagefaults() is there for the reason,
and if Linux KPI does not have a place to store the state, some replacement
must be implemented instead of silently corrupting the flags on enable.


Hi Konstantin,

Thank you for you input. I'll update these functions according to your suggestion. We can have a return value, even if Linux doesn't. It doesn't have to be exactly the same and then I can set a compiler attribute that the return value must be checked.

--HPS
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to