| Message ID | 1452864188-2417-15-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 15, 2016 at 01:22:53PM +0000, Ian Campbell wrote: > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > v6: Also discuss recovering the memory. > > v7: Further clarifications regarding forking based on ML discussions. > (Dropped Wei's ack) > --- > .../libs/foreignmemory/include/xenforeignmemory.h | 33 +++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h b/tools/libs/foreignmemory/include/xenforeignmemory.h > index 04ff548..a6d1bdb 100644 > --- a/tools/libs/foreignmemory/include/xenforeignmemory.h > +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h > @@ -32,13 +32,44 @@ typedef struct xentoollog_logger xentoollog_logger; > typedef struct xenforeignmemory_handle xenforeignmemory_handle; > > /* > - * Return a handle onto the hypercall driver. Logs errors. > + * Return a handle onto the foreign memory mapping driver. Logs errors. > + * > + * Note: After fork(2) a child process must not use any opened > + * foreignmemory handle inherited from their parent, nor access any > + * grant mapped areas associated with that handle. > + * > + * The child must open a new handle if they want to interact with > + * foreignmemory. > + * > + * Calling exec(2) in a child will safely (and reliably) reclaim any > + * resources which were allocated via a xenforeignmemory_handle in the > + * parent. > + * > + * A child which does not call exec(2) may safely call > + * xenforeignmemory_close() on a xenforeignmemory_handle inherited > + * from their parent. This will attempt to reclaim any resources > + * associated with that handle. Note that in some implementations this > + * reclamation may not be completely effective, in this case any > + * affected resources remain allocated. > + * > + * Calling xenforeignmemory_close() is the only safe operation on a > + * xenforeignmemory_handle which has been inherited. > */ > xenforeignmemory_handle *xenforeignmemory_open(xentoollog_logger *logger, > unsigned open_flags); > > /* > * Close a handle previously allocated with xenforeignmemory_open(). > + * > + * Under normal circumstances (i.e. not in the child after a fork) > + * xenforeignmemory_unmap() should be used on all mappings allocated "Should" according to RFC 2119 has the connotation of "there might be a valid reason to ignore such action". But after reading this passage I think we should use "must" here? Wei. > + * by xenforeignmemory_map() prior to closing the handle in order to > + * free up resources associated with those mappings. > + * > + * This is the only function which may be safely called on a > + * xenforeignmemory_handle in a child after a > + * fork. xenforeignmemory_unmap() must not be called under such > + * circumstances. > */ > int xenforeignmemory_close(xenforeignmemory_handle *fmem); > > -- > 2.1.4 >
On Tue, 2016-01-19 at 13:24 +0000, Wei Liu wrote: > On Fri, Jan 15, 2016 at 01:22:53PM +0000, Ian Campbell wrote: > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > --- > > v6: Also discuss recovering the memory. > > > > v7: Further clarifications regarding forking based on ML discussions. > > (Dropped Wei's ack) > > --- > > .../libs/foreignmemory/include/xenforeignmemory.h | 33 > > +++++++++++++++++++++- > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h > > b/tools/libs/foreignmemory/include/xenforeignmemory.h > > index 04ff548..a6d1bdb 100644 > > --- a/tools/libs/foreignmemory/include/xenforeignmemory.h > > +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h > > @@ -32,13 +32,44 @@ typedef struct xentoollog_logger xentoollog_logger; > > typedef struct xenforeignmemory_handle xenforeignmemory_handle; > > > > /* > > - * Return a handle onto the hypercall driver. Logs errors. > > + * Return a handle onto the foreign memory mapping driver. Logs > > errors. > > + * > > + * Note: After fork(2) a child process must not use any opened > > + * foreignmemory handle inherited from their parent, nor access any > > + * grant mapped areas associated with that handle. > > + * > > + * The child must open a new handle if they want to interact with > > + * foreignmemory. > > + * > > + * Calling exec(2) in a child will safely (and reliably) reclaim any > > + * resources which were allocated via a xenforeignmemory_handle in the > > + * parent. > > + * > > + * A child which does not call exec(2) may safely call > > + * xenforeignmemory_close() on a xenforeignmemory_handle inherited > > + * from their parent. This will attempt to reclaim any resources > > + * associated with that handle. Note that in some implementations this > > + * reclamation may not be completely effective, in this case any > > + * affected resources remain allocated. > > + * > > + * Calling xenforeignmemory_close() is the only safe operation on a > > + * xenforeignmemory_handle which has been inherited. > > */ > > xenforeignmemory_handle *xenforeignmemory_open(xentoollog_logger > > *logger, > > unsigned open_flags); > > > > /* > > * Close a handle previously allocated with xenforeignmemory_open(). > > + * > > + * Under normal circumstances (i.e. not in the child after a fork) > > + * xenforeignmemory_unmap() should be used on all mappings allocated > > "Should" according to RFC 2119 has the connotation of "there might be a > valid reason to ignore such action". But after reading this passage I > think we should use "must" here? RFC 2119 formally defines "SHOULD" not "should" (or "Should"), and in any case in order to be subject to those formal definitions a document would need to explicitly reference RFC 2119. I think most readers of normal English prose would probably take "must" and "should" to mean mostly the same thing. If there are other reasons to resend I don't mind switching it to must, but I don't think it is worth a resend of this mega-series for what is a minor semantic quibble. Ian.
On Tue, Jan 19, 2016 at 01:34:58PM +0000, Ian Campbell wrote: > On Tue, 2016-01-19 at 13:24 +0000, Wei Liu wrote: > > On Fri, Jan 15, 2016 at 01:22:53PM +0000, Ian Campbell wrote: > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > > --- > > > v6: Also discuss recovering the memory. > > > > > > v7: Further clarifications regarding forking based on ML discussions. > > > (Dropped Wei's ack) > > > --- > > > .../libs/foreignmemory/include/xenforeignmemory.h | 33 > > > +++++++++++++++++++++- > > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h > > > b/tools/libs/foreignmemory/include/xenforeignmemory.h > > > index 04ff548..a6d1bdb 100644 > > > --- a/tools/libs/foreignmemory/include/xenforeignmemory.h > > > +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h > > > @@ -32,13 +32,44 @@ typedef struct xentoollog_logger xentoollog_logger; > > > typedef struct xenforeignmemory_handle xenforeignmemory_handle; > > > > > > /* > > > - * Return a handle onto the hypercall driver. Logs errors. > > > + * Return a handle onto the foreign memory mapping driver. Logs > > > errors. > > > + * > > > + * Note: After fork(2) a child process must not use any opened > > > + * foreignmemory handle inherited from their parent, nor access any > > > + * grant mapped areas associated with that handle. > > > + * > > > + * The child must open a new handle if they want to interact with > > > + * foreignmemory. > > > + * > > > + * Calling exec(2) in a child will safely (and reliably) reclaim any > > > + * resources which were allocated via a xenforeignmemory_handle in the > > > + * parent. > > > + * > > > + * A child which does not call exec(2) may safely call > > > + * xenforeignmemory_close() on a xenforeignmemory_handle inherited > > > + * from their parent. This will attempt to reclaim any resources > > > + * associated with that handle. Note that in some implementations this > > > + * reclamation may not be completely effective, in this case any > > > + * affected resources remain allocated. > > > + * > > > + * Calling xenforeignmemory_close() is the only safe operation on a > > > + * xenforeignmemory_handle which has been inherited. > > > */ > > > xenforeignmemory_handle *xenforeignmemory_open(xentoollog_logger > > > *logger, > > > unsigned open_flags); > > > > > > /* > > > * Close a handle previously allocated with xenforeignmemory_open(). > > > + * > > > + * Under normal circumstances (i.e. not in the child after a fork) > > > + * xenforeignmemory_unmap() should be used on all mappings allocated > > > > "Should" according to RFC 2119 has the connotation of "there might be a > > valid reason to ignore such action". But after reading this passage I > > think we should use "must" here? > > RFC 2119 formally defines "SHOULD" not "should" (or "Should"), and in any > case in order to be subject to those formal definitions a document would > need to explicitly reference RFC 2119. > > I think most readers of normal English prose would probably take "must" and > "should" to mean mostly the same thing. > > If there are other reasons to resend I don't mind switching it to must, but > I don't think it is worth a resend of this mega-series for what is a minor > semantic quibble. > No need to resend then. Acked-by: Wei Liu <wei.liu2@citrix.com> Wei. > Ian.
diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h b/tools/libs/foreignmemory/include/xenforeignmemory.h index 04ff548..a6d1bdb 100644 --- a/tools/libs/foreignmemory/include/xenforeignmemory.h +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h @@ -32,13 +32,44 @@ typedef struct xentoollog_logger xentoollog_logger; typedef struct xenforeignmemory_handle xenforeignmemory_handle; /* - * Return a handle onto the hypercall driver. Logs errors. + * Return a handle onto the foreign memory mapping driver. Logs errors. + * + * Note: After fork(2) a child process must not use any opened + * foreignmemory handle inherited from their parent, nor access any + * grant mapped areas associated with that handle. + * + * The child must open a new handle if they want to interact with + * foreignmemory. + * + * Calling exec(2) in a child will safely (and reliably) reclaim any + * resources which were allocated via a xenforeignmemory_handle in the + * parent. + * + * A child which does not call exec(2) may safely call + * xenforeignmemory_close() on a xenforeignmemory_handle inherited + * from their parent. This will attempt to reclaim any resources + * associated with that handle. Note that in some implementations this + * reclamation may not be completely effective, in this case any + * affected resources remain allocated. + * + * Calling xenforeignmemory_close() is the only safe operation on a + * xenforeignmemory_handle which has been inherited. */ xenforeignmemory_handle *xenforeignmemory_open(xentoollog_logger *logger, unsigned open_flags); /* * Close a handle previously allocated with xenforeignmemory_open(). + * + * Under normal circumstances (i.e. not in the child after a fork) + * xenforeignmemory_unmap() should be used on all mappings allocated + * by xenforeignmemory_map() prior to closing the handle in order to + * free up resources associated with those mappings. + * + * This is the only function which may be safely called on a + * xenforeignmemory_handle in a child after a + * fork. xenforeignmemory_unmap() must not be called under such + * circumstances. */ int xenforeignmemory_close(xenforeignmemory_handle *fmem);
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- v6: Also discuss recovering the memory. v7: Further clarifications regarding forking based on ML discussions. (Dropped Wei's ack) --- .../libs/foreignmemory/include/xenforeignmemory.h | 33 +++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)