Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 Jul 2015 16:05:25 +0000 (UTC)
From:      Luigi Rizzo <luigi@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r285359 - head/sys/dev/netmap
Message-ID:  <201507101605.t6AG5PH6053629@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: luigi
Date: Fri Jul 10 16:05:24 2015
New Revision: 285359
URL: https://svnweb.freebsd.org/changeset/base/285359

Log:
  staticize functions only used in netmap.c
  (detected by jenkins run with gcc 4.9)
  
  Update documentation on the use of netmap_priv_d,
  rename the refcount and use the same structure in
  FreeBSD and linux
  
  No functional changes.

Modified:
  head/sys/dev/netmap/netmap.c
  head/sys/dev/netmap/netmap_freebsd.c
  head/sys/dev/netmap/netmap_kern.h
  head/sys/dev/netmap/netmap_vale.c

Modified: head/sys/dev/netmap/netmap.c
==============================================================================
--- head/sys/dev/netmap/netmap.c	Fri Jul 10 14:39:46 2015	(r285358)
+++ head/sys/dev/netmap/netmap.c	Fri Jul 10 16:05:24 2015	(r285359)
@@ -726,6 +726,9 @@ netmap_update_config(struct netmap_adapt
 	return 1;
 }
 
+static void netmap_txsync_to_host(struct netmap_adapter *na);
+static int netmap_rxsync_from_host(struct netmap_adapter *na, struct thread *td, void *pwait);
+
 /* kring->nm_sync callback for the host tx ring */
 static int
 netmap_txsync_to_host_compat(struct netmap_kring *kring, int flags)
@@ -959,11 +962,12 @@ nm_si_user(struct netmap_priv_d *priv, e
 }
 
 /*
- * Destructor of the netmap_priv_d, called when the fd has
- * no active open() and mmap().
- * Undo all the things done by NIOCREGIF.
+ * Destructor of the netmap_priv_d, called when the fd is closed
+ * Action: undo all the things done by NIOCREGIF,
+ * On FreeBSD we need to track whether there are active mmap()s,
+ * and we use np_active_mmaps for that. On linux, the field is always 0.
+ * Return: 1 if we can free priv, 0 otherwise.
  *
- * returns 1 if this is the last instance and we can free priv
  */
 /* call with NMG_LOCK held */
 int
@@ -971,17 +975,13 @@ netmap_dtor_locked(struct netmap_priv_d 
 {
 	struct netmap_adapter *na = priv->np_na;
 
-#ifdef __FreeBSD__
-	/*
-	 * np_refcount is the number of active mmaps on
-	 * this file descriptor
-	 */
-	if (--priv->np_refcount > 0) {
+	/* number of active mmaps on this fd (FreeBSD only) */
+	if (--priv->np_refs > 0) {
 		return 0;
 	}
-#endif /* __FreeBSD__ */
+
 	if (!na) {
-	    return 1; //XXX is it correct?
+		return 1; //XXX is it correct?
 	}
 	netmap_do_unregif(priv);
 	netmap_adapter_put(na);
@@ -1139,7 +1139,7 @@ netmap_sw_to_nic(struct netmap_adapter *
  * can be among multiple user threads erroneously calling
  * this routine concurrently.
  */
-void
+static void
 netmap_txsync_to_host(struct netmap_adapter *na)
 {
 	struct netmap_kring *kring = &na->tx_rings[na->num_tx_rings];
@@ -1177,7 +1177,7 @@ netmap_txsync_to_host(struct netmap_adap
  * returns the number of packets delivered to tx queues in
  * transparent mode, or a negative value if error
  */
-int
+static int
 netmap_rxsync_from_host(struct netmap_adapter *na, struct thread *td, void *pwait)
 {
 	struct netmap_kring *kring = &na->rx_rings[na->num_rx_rings];

Modified: head/sys/dev/netmap/netmap_freebsd.c
==============================================================================
--- head/sys/dev/netmap/netmap_freebsd.c	Fri Jul 10 14:39:46 2015	(r285358)
+++ head/sys/dev/netmap/netmap_freebsd.c	Fri Jul 10 16:05:24 2015	(r285359)
@@ -576,7 +576,7 @@ netmap_mmap_single(struct cdev *cdev, vm
 		goto err_unlock;
 	}
 	vmh->priv = priv;
-	priv->np_refcount++;
+	priv->np_refs++;
 	NMG_UNLOCK();
 
 	obj = cdev_pager_allocate(vmh, OBJT_DEVICE,
@@ -593,7 +593,7 @@ netmap_mmap_single(struct cdev *cdev, vm
 
 err_deref:
 	NMG_LOCK();
-	priv->np_refcount--;
+	priv->np_refs--;
 err_unlock:
 	NMG_UNLOCK();
 // err:
@@ -602,14 +602,14 @@ err_unlock:
 }
 
 /*
- * netmap_close() is called on every close(), but we do not need to do
- * anything at that moment, since the process may have other open file
- * descriptors for /dev/netmap. Instead, we pass netmap_dtor() to
+ * On FreeBSD the close routine is only called on the last close on
+ * the device (/dev/netmap) so we cannot do anything useful.
+ * To track close() on individual file descriptors we pass netmap_dtor() to
  * devfs_set_cdevpriv() on open(). The FreeBSD kernel will call the destructor
  * when the last fd pointing to the device is closed. 
  *
- * Unfortunately, FreeBSD does not automatically track active mmap()s on an fd,
- * so we have to track them by ourselvesi (see above). The result is that
+ * Note that FreeBSD does not even munmap() on close() so we also have
+ * to track mmap() ourselves, and postpone the call to
  * netmap_dtor() is called when the process has no open fds and no active
  * memory maps on /dev/netmap, as in linux.
  */
@@ -634,19 +634,15 @@ netmap_open(struct cdev *dev, int oflags
 	(void)devtype;
 	(void)td;
 
-	// XXX wait or nowait ?
 	priv = malloc(sizeof(struct netmap_priv_d), M_DEVBUF,
 			      M_NOWAIT | M_ZERO);
 	if (priv == NULL)
 		return ENOMEM;
-
 	error = devfs_set_cdevpriv(priv, netmap_dtor);
-	if (error)
-	        return error;
-
-	priv->np_refcount = 1;
-
-	return 0;
+	if (error) {
+		free(priv, M_DEVBUF);
+	}
+	return error;
 }
 
 /******************** kqueue support ****************/

Modified: head/sys/dev/netmap/netmap_kern.h
==============================================================================
--- head/sys/dev/netmap/netmap_kern.h	Fri Jul 10 14:39:46 2015	(r285358)
+++ head/sys/dev/netmap/netmap_kern.h	Fri Jul 10 16:05:24 2015	(r285359)
@@ -925,8 +925,6 @@ static __inline void nm_kr_get(struct ne
 }
 
 
-
-
 /*
  * The following functions are used by individual drivers to
  * support netmap operation.
@@ -1079,8 +1077,6 @@ int netmap_krings_create(struct netmap_a
  * been created using netmap_krings_create
  */
 void netmap_krings_delete(struct netmap_adapter *na);
-int netmap_rxsync_from_host(struct netmap_adapter *na, struct thread *td, void *pwait);
-
 
 /* set the stopped/enabled status of ring
  * When stopping, they also wait for all current activity on the ring to
@@ -1094,14 +1090,10 @@ void netmap_set_all_rings(struct netmap_
 void netmap_disable_all_rings(struct ifnet *);
 void netmap_enable_all_rings(struct ifnet *);
 
-int netmap_rxsync_from_host(struct netmap_adapter *na, struct thread *td, void *pwait);
-
-int
-netmap_do_regif(struct netmap_priv_d *priv, struct netmap_adapter *na,
+int netmap_do_regif(struct netmap_priv_d *priv, struct netmap_adapter *na,
 	uint16_t ringid, uint32_t flags);
 
 
-
 u_int nm_bound_var(u_int *v, u_int dflt, u_int lo, u_int hi, const char *msg);
 int netmap_get_na(struct nmreq *nmr, struct netmap_adapter **na, int create);
 int netmap_get_hw_na(struct ifnet *ifp, struct netmap_adapter **na);
@@ -1482,37 +1474,21 @@ PNMB(struct netmap_adapter *na, struct n
 	return ret;
 }
 
-/* Generic version of NMB, which uses device-specific memory. */
-
-
-
-void netmap_txsync_to_host(struct netmap_adapter *na);
-
 
 /*
- * Structure associated to each thread which registered an interface.
- *
- * The first 4 fields of this structure are written by NIOCREGIF and
- * read by poll() and NIOC?XSYNC.
- *
- * There is low contention among writers (a correct user program
- * should have none) and among writers and readers, so we use a
- * single global lock to protect the structure initialization;
- * since initialization involves the allocation of memory,
- * we reuse the memory allocator lock.
- *
- * Read access to the structure is lock free. Readers must check that
- * np_nifp is not NULL before using the other fields.
- * If np_nifp is NULL initialization has not been performed,
- * so they should return an error to userspace.
- *
- * The ref_done field (XXX ?) is used to regulate access to the refcount in the
- * memory allocator. The refcount must be incremented at most once for
- * each open("/dev/netmap"). The increment is performed by the first
- * function that calls netmap_get_memory() (currently called by
- * mmap(), NIOCGINFO and NIOCREGIF).
- * If the refcount is incremented, it is then decremented when the
- * private structure is destroyed.
+ * Structure associated to each netmap file descriptor.
+ * It is created on open and left unbound (np_nifp == NULL).
+ * A successful NIOCREGIF will set np_nifp and the first few fields;
+ * this is protected by a global lock (NMG_LOCK) due to low contention.
+ *
+ * np_refs counts the number of references to the structure: one for the fd,
+ * plus (on FreeBSD) one for each active mmap which we track ourselves
+ * (they are not unmapped on close(), unlike linux).
+ * np_refs is protected by NMG_LOCK.
+ *
+ * Read access to the structure is lock free, because ni_nifp once set
+ * can only go to 0 when nobody is using the entry anymore. Readers
+ * must check that np_nifp != NULL before using the other fields.
  */
 struct netmap_priv_d {
 	struct netmap_if * volatile np_nifp;	/* netmap if descriptor. */
@@ -1523,8 +1499,7 @@ struct netmap_priv_d {
 			np_qlast[NR_TXRX]; /* range of tx/rx rings to scan */
 	uint16_t	np_txpoll;	/* XXX and also np_rxpoll ? */
 
-	/* np_refcount is only used on FreeBSD */
-	int		np_refcount;	/* use with NMG_LOCK held */
+	int		np_refs;	/* use with NMG_LOCK held */
 
 	/* pointers to the selinfo to be used for selrecord.
 	 * Either the local or the global one depending on the

Modified: head/sys/dev/netmap/netmap_vale.c
==============================================================================
--- head/sys/dev/netmap/netmap_vale.c	Fri Jul 10 14:39:46 2015	(r285358)
+++ head/sys/dev/netmap/netmap_vale.c	Fri Jul 10 16:05:24 2015	(r285359)
@@ -1931,7 +1931,6 @@ netmap_bwrap_intr_notify(struct netmap_k
 	struct netmap_adapter *na = kring->na;
 	struct netmap_bwrap_adapter *bna = na->na_private;
 	struct netmap_kring *bkring;
-	struct netmap_ring *ring;
 	struct netmap_vp_adapter *vpna = &bna->up;
 	u_int ring_nr = kring->ring_id;
 	int error = 0;
@@ -1943,7 +1942,6 @@ netmap_bwrap_intr_notify(struct netmap_k
 		return 0;
 
 	bkring = &vpna->up.tx_rings[ring_nr];
-	ring = kring->ring; /* == kbkring->ring */
 
 	/* make sure the ring is not disabled */
 	if (nm_kr_tryget(kring))



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201507101605.t6AG5PH6053629>