- GRU Driver V3 fixes to resolve code review comments
Subsystems (2)
PATCH 1/1 - GRU Driver V3 fixes to resolve code review comments
Fixes problems identified in a code review:
- add comment with high level dscription of the GRU
- prepend "gru_" to all global names
- delete unused function
- couple of trivial bug fixes
--- drivers/misc/sgi-gru/gru_instructions.h | 10 ---- drivers/misc/sgi-gru/grufile.c | 8 ++- drivers/misc/sgi-gru/grukservices.c | 6 +- drivers/misc/sgi-gru/grumain.c | 16 ++++-- drivers/misc/sgi-gru/gruprocfs.c | 4 - drivers/misc/sgi-gru/grutables.h | 74 +++++++++++++++++++++++++++++--- drivers/misc/sgi-gru/grutlbpurge.c | 4 + 7 files changed, 93 insertions(+), 29 deletions(-)
drivers/misc/sgi-gru/gru_instructions.h +0 -10
@@ -285,16 +285,6 @@ __opword(unsigned char opcode, unsigned }/* -* Prefetch a cacheline. Fetch is unconditional. Must page fault if -* no valid TLB entry is found. -* ??? should I use actual "load" or hardware prefetch??? -*/ -static inline void gru_prefetch (void *p) -{ - *(volatile char *)p; -} - -/* * Architecture specific intrinsics */ static inline void gru_flush_cache (void *p)
drivers/misc/sgi-gru/grufile.c +6 -2
@@ -112,6 +112,10 @@ static int gru_file_mmap(struct file *fiif ((vma->vm_flags & (VM_SHARED |VM_WRITE )) != (VM_SHARED |VM_WRITE ))return -EPERM ; +if (vma->vm_start & (GRU_GSEG_PAGESIZE -1 ) || + vma->vm_end & (GRU_GSEG_PAGESIZE -1 )) +return -EINVAL ; + vma->vm_flags |= (VM_IO |VM_DONTCOPY |VM_LOCKED |VM_DONTEXPAND |VM_PFNMAP |VM_RESERVED );
@@ -471,8 +475,8 @@ struct vm_operations_struct gru_vm_ops =module_init (gru_init);module_exit (gru_exit); -module_param (options, ulong,0644 ); -MODULE_PARM_DESC (options,"Various debug options" ); +module_param (gru_options, ulong,0644 ); +MODULE_PARM_DESC (gru_options,"Various debug options" );MODULE_AUTHOR ("Silicon Graphics, Inc." );MODULE_LICENSE ("GPL" );
drivers/misc/sgi-gru/grukservices.c +3 -3
@@ -638,11 +638,11 @@ int gru_kservices_init(struct gru_state cpus_possible =uv_blade_nr_possible_cpus (gru->gs_blade_id); num =GRU_NUM_KERNEL_CBR * cpus_possible; - cbr_map =reserve_gru_cb_resources (gru,GRU_CB_COUNT_TO_AU (num),NULL ); + cbr_map =gru_reserve_cb_resources (gru,GRU_CB_COUNT_TO_AU (num),NULL ); gru->gs_reserved_cbrs += num; num =GRU_NUM_KERNEL_DSR_BYTES * cpus_possible; - dsr_map =reserve_gru_ds_resources (gru,GRU_DS_BYTES_TO_AU (num),NULL ); + dsr_map =gru_reserve_ds_resources (gru,GRU_DS_BYTES_TO_AU (num),NULL ); gru->gs_reserved_dsr_bytes += num; gru->gs_active_contexts++;
@@ -673,7 +673,7 @@ int gru_kservices_init(struct gru_state }unlock_cch_handle (cch); -if (options &GRU_QUICKLOOK ) +if (gru_options &GRU_QUICKLOOK )quicktest (gru);return 0 ; }
drivers/misc/sgi-gru/grumain.c +10 -6
@@ -22,7 +22,7 @@#include "grutables.h" #include "gruhandles.h" -unsigned long options__read_mostly ; +unsigned long gru_options__read_mostly ;static struct device_driver gru_driver = { .name ="gru"
@@ -163,14 +163,14 @@ static unsigned long reserve_resources(ureturn bits; } -unsigned long reserve_gru_cb_resources (struct gru_state *gru,int cbr_au_count, +unsigned long gru_reserve_cb_resources (struct gru_state *gru,int cbr_au_count,char *cbmap) {return reserve_resources (&gru->gs_cbr_map, cbr_au_count,GRU_CBR_AU , cbmap); } -unsigned long reserve_gru_ds_resources (struct gru_state *gru,int dsr_au_count, +unsigned long gru_reserve_ds_resources (struct gru_state *gru,int dsr_au_count,char *dsmap) { return reserve_resources(&gru->gs_dsr_map, dsr_au_count, GRU_DSR_AU,
@@ -182,10 +182,10 @@ static void reserve_gru_resources(struct { gru->gs_active_contexts++; gts->ts_cbr_map = -reserve_gru_cb_resources (gru, gts->ts_cbr_au_count, +gru_reserve_cb_resources (gru, gts->ts_cbr_au_count, gts->ts_cbr_idx); gts->ts_dsr_map = -reserve_gru_ds_resources (gru, gts->ts_dsr_au_count,NULL ); +gru_reserve_ds_resources (gru, gts->ts_dsr_au_count,NULL ); } static void free_gru_resources(struct gru_state *gru,
@@ -416,6 +416,7 @@ static void gru_free_gru_context(struct/* * Prefetching cachelines help hardware performance. +* (Strictly a performance enhancement. Not functionally required). */ static void prefetch_data (void *p,int num,int stride) {
@@ -746,6 +747,8 @@ again:* gru_nopage * * Map the user's GRU segment +* +* Note: gru segments alway mmaped on GRU_GSEG_PAGESIZE boundaries. */ int gru_fault (struct vm_area_struct *vma,struct vm_fault *vmf) {
@@ -757,6 +760,7 @@ int gru_fault(struct vm_area_struct *vma vma, vaddr, GSEG_BASE(vaddr));STAT (nopfn); +/* The following check ensures vaddr is a valid address in the VMA */ gts =gru_find_thread_state (vma,TSID (vaddr, vma));if (!gts)return VM_FAULT_SIGBUS ;
@@ -775,7 +779,7 @@ again: }if (!gts->ts_gru) { -while (!gru_assign_gru_context (gts)) { +if (!gru_assign_gru_context (gts)) {mutex_unlock (>s->ts_ctxlock);preempt_enable ();schedule_timeout (GRU_ASSIGN_DELAY );/* true hack ZZZ */
drivers/misc/sgi-gru/gruprocfs.c +2 -2
@@ -122,7 +122,7 @@ static ssize_t statistics_write(struct fstatic int options_show (struct seq_file *s,void *p) { -seq_printf (s,"0x%lx\n" , options); +seq_printf (s,"0x%lx\n" , gru_options);return 0 ; }
@@ -136,7 +136,7 @@ static ssize_t options_write(struct file (buf, userbuf, count < sizeof(buf) ? count : sizeof(buf))) return -EFAULT;if (!strict_strtoul (buf,10 , &val)) - options = val; + gru_options = val;return count; }
drivers/misc/sgi-gru/grutables.h +69 -5
104 lines not on this page · raw message
drivers/misc/sgi-gru/grutlbpurge.c +3 -1
@@ -242,7 +242,9 @@ static void gru_invalidate_range_end(str struct gru_mm_struct *gms = container_of(mn, struct gru_mm_struct, ms_notifier); -atomic_dec (&gms->ms_range_active); +/* ..._and_test() provides needed barrier */ + (void )atomic_dec_and_test (&gms->ms_range_active); +wake_up_all (&gms->ms_wait_queue);gru_dbg (grudev,"gms %p, start 0x%lx, end 0x%lx\n" , gms, start, end); }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Jack Steiner
Index:
linux/drivers/misc/sgi-gru/grutlbpurge.c
===================================================================
---linux.orig/drivers/misc/sgi-gru/grutlbpurge.c2008-07-28
14:52:26.000000000 -0500 +++linux/drivers/misc/sgi-gru/grutlbpurge.c2008-07-28 14:52:54.000000000
-0500 @@ -242,7 +242,9 @@ static void gru_invalidate_range_end(str
drivers/misc/sgi-gru/grutlbpurge.c static void gru_invalidate_range_end(str · line 248
Quoted 8 lines
drivers/misc/sgi-gru/grutlbpurge.c +3
-1struct gru_mm_struct *gms = container_of(mn, struct gru_mm_struct, ms_notifier); -atomic_dec (&gms->ms_range_active); +/* ..._and_test() provides needed barrier */ + (void )atomic_dec_and_test (&gms->ms_range_active); +wake_up_all (&gms->ms_wait_queue);
What about
if (atomic_dec_and_test())
wake_up_all()
?
Also, I would prefer either a comment to say why the increment of ms_range_active is safe, or an smp_mb__after_atomic_inc() for it too.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>