Re: extent_supply_region_descriptor

2014-01-20 Thread Miod Vallat
 In order to make the sparc64 iommu code mpsafe, I need to make sure
 the extent manager can be used in an mpsafe manner.  The current code
 isn't really safe since the extent manager needs to allocate region
 descriptors whenever we do a bus_dmamap_load().  The diff below adds a
 function to provide the extent manager with a region descriptor such
 that the next extent_alloc_xxx() call can use that one instead of
 allocating a new one.
 
 I deliberately chose to add a seperate API, since
 extent_alloc_subregion() already has too many arguments.
 
 The 2nd diff shows how I use this in the sparc64 iommu code.
 
 opinions?

This is ugly. Better introduce an extent_alloc_with_descr() or
extent_alloc_static() interface which is similar to extent_alloc(), but
receives the extent descriptor as an added argument.



Re: extent_supply_region_descriptor

2014-01-20 Thread Mark Kettenis
 This is ugly. Better introduce an extent_alloc_with_descr() or
 extent_alloc_static() interface which is similar to extent_alloc(), but
 receives the extent descriptor as an added argument.

Fair enough; seems this is indeed a better interface as it doesn't
require the use of a fixed extent.

Here's a new diff.  I'll do the man page before committing this.

ok?


Index: sys/extent.h
===
RCS file: /cvs/src/sys/sys/extent.h,v
retrieving revision 1.12
diff -u -p -r1.12 extent.h
--- sys/extent.h19 Apr 2009 15:26:52 -  1.12
+++ sys/extent.h21 Jan 2014 00:28:12 -
@@ -44,6 +44,7 @@ struct extent_region {
 
 /* er_flags */
 #define ER_ALLOC   0x01/* region descriptor dynamically allocated */
+#define ER_DISCARD 0x02/* discard region descriptor after use */
 
 struct extent {
char*ex_name;   /* name of extent */
@@ -105,6 +106,9 @@ struct  extent *extent_create(char *, u_l
 void   extent_destroy(struct extent *);
 intextent_alloc_subregion(struct extent *, u_long, u_long,
u_long, u_long, u_long, u_long, int, u_long *);
+intextent_alloc_subregion_with_descr(struct extent *, u_long, u_long,
+   u_long, u_long, u_long, u_long, int, struct extent_region *,
+   u_long *);
 intextent_alloc_region(struct extent *, u_long, u_long, int);
 intextent_free(struct extent *, u_long, u_long, int);
 void   extent_print(struct extent *);
Index: kern/subr_extent.c
===
RCS file: /cvs/src/sys/kern/subr_extent.c,v
retrieving revision 1.48
diff -u -p -r1.48 subr_extent.c
--- kern/subr_extent.c  8 Aug 2013 23:25:06 -   1.48
+++ kern/subr_extent.c  21 Jan 2014 00:32:35 -
@@ -78,6 +78,8 @@ staticvoid extent_insert_and_optimize(s
 static struct extent_region *extent_alloc_region_descriptor(struct extent *, 
int);
 static void extent_free_region_descriptor(struct extent *,
struct extent_region *);
+intextent_do_alloc(struct extent *, u_long, u_long, u_long, u_long,
+   u_long, u_long, int, struct extent_region *, u_long *);
 
 /*
  * Shortcut to align to an arbitrary power-of-two boundary.
@@ -580,11 +582,11 @@ extent_alloc_region(struct extent *ex, u
  * a power of 2.
  */
 int
-extent_alloc_subregion(struct extent *ex, u_long substart, u_long subend,
+extent_do_alloc(struct extent *ex, u_long substart, u_long subend,
 u_long size, u_long alignment, u_long skew, u_long boundary, int flags,
-u_long *result)
+struct extent_region *myrp, u_long *result)
 {
-   struct extent_region *rp, *myrp, *last, *bestlast;
+   struct extent_region *rp, *last, *bestlast;
u_long newstart, newend, exend, beststart, bestovh, ovh;
u_long dontcross;
int error;
@@ -593,6 +595,8 @@ extent_alloc_subregion(struct extent *ex
/* Check arguments. */
if (ex == NULL)
panic(%s: NULL extent, __func__);
+   if (myrp == NULL)
+   panic(%s: NULL region descriptor, __func__);
if (result == NULL)
panic(%s: NULL result pointer, __func__);
if ((substart  ex-ex_start) || (substart  ex-ex_end) ||
@@ -617,18 +621,6 @@ extent_alloc_subregion(struct extent *ex
}
 #endif
 
-   /*
-* Allocate the region descriptor.  It will be freed later
-* if we can coalesce with another region.
-*/
-   myrp = extent_alloc_region_descriptor(ex, flags);
-   if (myrp == NULL) {
-#ifdef DIAGNOSTIC
-   printf(%s: can't allocate region descriptor\n, __func__);
-#endif
-   return (ENOMEM);
-   }
-
  alloc_start:
/*
 * Keep a pointer to the last region we looked at so
@@ -927,6 +919,41 @@ skip:
 }
 
 int
+extent_alloc_subregion(struct extent *ex, u_long substart, u_long subend,
+u_long size, u_long alignment, u_long skew, u_long boundary, int flags,
+u_long *result)
+{
+   struct extent_region *rp;
+
+   /*
+* Allocate the region descriptor.  It will be freed later
+* if we can coalesce with another region.
+*/
+   rp = extent_alloc_region_descriptor(ex, flags);
+   if (rp == NULL) {
+#ifdef DIAGNOSTIC
+   printf(%s: can't allocate region descriptor\n, __func__);
+#endif
+   return (ENOMEM);
+   }
+
+   return extent_do_alloc(ex, substart, subend, size, alignment, skew,
+   boundary, flags, rp, result);
+}
+
+int
+extent_alloc_subregion_with_descr(struct extent *ex, u_long substart,
+u_long subend, u_long size, u_long alignment, u_long skew,
+u_long boundary, int flags, struct extent_region *rp, u_long *result)
+{
+   KASSERT(ex-ex_flags  EXF_NOCOALESCE);
+
+   rp-er_flags = ER_DISCARD;
+   return extent_do_alloc(ex, substart, subend, size, alignment, skew,
+   boundary, flags, rp, result);
+}
+
+int
 

extent_supply_region_descriptor

2014-01-19 Thread Mark Kettenis
In order to make the sparc64 iommu code mpsafe, I need to make sure
the extent manager can be used in an mpsafe manner.  The current code
isn't really safe since the extent manager needs to allocate region
descriptors whenever we do a bus_dmamap_load().  The diff below adds a
function to provide the extent manager with a region descriptor such
that the next extent_alloc_xxx() call can use that one instead of
allocating a new one.

I deliberately chose to add a seperate API, since
extent_alloc_subregion() already has too many arguments.

The 2nd diff shows how I use this in the sparc64 iommu code.

opinions?

Index: sys/extent.h
===
RCS file: /cvs/src/sys/sys/extent.h,v
retrieving revision 1.12
diff -u -p -r1.12 extent.h
--- sys/extent.h19 Apr 2009 15:26:52 -  1.12
+++ sys/extent.h20 Jan 2014 04:01:45 -
@@ -44,6 +44,7 @@ struct extent_region {
 
 /* er_flags */
 #define ER_ALLOC   0x01/* region descriptor dynamically allocated */
+#define ER_DISCARD 0x02/* discard region descriptor after use */
 
 struct extent {
char*ex_name;   /* name of extent */
@@ -101,13 +102,15 @@ struct extent_fixed {
 void extent_print_all(void);
 
 struct extent *extent_create(char *, u_long, u_long, int,
-   caddr_t, size_t, int);
+   void *, size_t, int);
 void   extent_destroy(struct extent *);
 intextent_alloc_subregion(struct extent *, u_long, u_long,
u_long, u_long, u_long, u_long, int, u_long *);
 intextent_alloc_region(struct extent *, u_long, u_long, int);
 intextent_free(struct extent *, u_long, u_long, int);
 void   extent_print(struct extent *);
+void   extent_supply_region_descriptor(struct extent *,
+   struct extent_region *);
 
 /* Simple case of extent_alloc_subregion() */
 #define extent_alloc(_ex, _size, _alignment, _skew, _boundary, \
Index: kern/subr_extent.c
===
RCS file: /cvs/src/sys/kern/subr_extent.c,v
retrieving revision 1.48
diff -u -p -r1.48 subr_extent.c
--- kern/subr_extent.c  8 Aug 2013 23:25:06 -   1.48
+++ kern/subr_extent.c  20 Jan 2014 04:01:45 -
@@ -157,7 +157,7 @@ extent_print_all(void)
  * Allocate and initialize an extent map.
  */
 struct extent *
-extent_create(char *name, u_long start, u_long end, int mtype, caddr_t storage,
+extent_create(char *name, u_long start, u_long end, int mtype, void *storage,
 size_t storagesize, int flags)
 {
struct extent *ex;
@@ -,6 +,9 @@ extent_alloc_region_descriptor(struct ex
 static void
 extent_free_region_descriptor(struct extent *ex, struct extent_region *rp)
 {
+   if (rp-er_flags  ER_DISCARD)
+   return;
+
if (ex-ex_flags  EXF_FIXED) {
struct extent_fixed *fex = (struct extent_fixed *)ex;
 
@@ -1149,7 +1152,17 @@ extent_free_region_descriptor(struct ext
pool_put(ex_region_pl, rp);
 }
 
-   
+void
+extent_supply_region_descriptor(struct extent *ex, struct extent_region *rp)
+{
+   struct extent_fixed *fex = (struct extent_fixed *)ex;
+
+   KASSERT(ex-ex_flags  EXF_FIXED);
+
+   rp-er_flags = ER_DISCARD;
+   LIST_INSERT_HEAD(fex-fex_freelist, rp, er_link);
+}
+
 #if defined(DIAGNOSTIC) || defined(DDB) || !defined(_KERNEL)
 
 void


Index: arch/sparc64/dev/iommu.c
===
RCS file: /cvs/src/sys/arch/sparc64/dev/iommu.c,v
retrieving revision 1.66
diff -u -p -r1.66 iommu.c
--- arch/sparc64/dev/iommu.c15 Jan 2013 03:14:01 -  1.66
+++ arch/sparc64/dev/iommu.c20 Jan 2014 04:03:41 -
@@ -226,7 +226,7 @@ iommu_init(char *name, struct iommu_stat
 #endif
is-is_dvmamap = extent_create(name,
is-is_dvmabase, (u_long)is-is_dvmaend + 1,
-   M_DEVBUF, 0, 0, EX_NOWAIT);
+   M_DEVBUF, is-is_fex, sizeof(is-is_fex), EX_NOCOALESCE);
mtx_init(is-is_mtx, IPL_HIGH);
 
/*
@@ -749,6 +749,7 @@ iommu_dvmamap_load(bus_dma_tag_t t, bus_
 * If our segment size is larger than the boundary we need to 
 * split the transfer up into little pieces ourselves.
 */
+   extent_supply_region_descriptor(is-is_dvmamap, ims-ims_er);
err = extent_alloc_subregion(is-is_dvmamap, sgstart, sgend,
sgsize, align, 0, (sgsize  boundary) ? 0 : boundary, 
EX_NOWAIT | EX_BOUNDZERO, (u_long *)dvmaddr);
@@ -956,6 +957,7 @@ iommu_dvmamap_load_raw(bus_dma_tag_t t, 
 * If our segment size is larger than the boundary we need to 
 * split the transfer up into little pieces ourselves.
 */
+   extent_supply_region_descriptor(is-is_dvmamap, ims-ims_er);
err = extent_alloc_subregion(is-is_dvmamap, sgstart, sgend,
sgsize, align, 0, (sgsize  boundary) ? 0 : boundary, 
EX_NOWAIT | EX_BOUNDZERO, (u_long *)dvmaddr);
Index: arch/sparc64