On Fri, 2009-03-20 at 14:47 -0400, Oren Laadan wrote:
> diff --git a/checkpoint/checkpoint_mem.h b/checkpoint/checkpoint_mem.h
> index de1d4c8..3157dde 100644
> --- a/checkpoint/checkpoint_mem.h
> +++ b/checkpoint/checkpoint_mem.h
> @@ -43,4 +43,22 @@ static inline int cr_pgarr_nr_free(struct cr_pgarr *pgarr)
> return CR_PGARR_TOTAL - pgarr->nr_used;
> }
>
> +/*
> + * This probably belongs in include/linux/mm.h
> + */
> +
> +/* Flag allocation requirements to shmem_getpage and shmem_swp_alloc */
> +enum sgp_type {
> + SGP_READ, /* don't exceed i_size, don't allocate page */
> + SGP_CACHE, /* don't exceed i_size, may allocate page */
> + SGP_DIRTY, /* like SGP_CACHE, but set new page dirty */
> + SGP_WRITE, /* may exceed i_size, may allocate page */
> +};
> +
> +int shmem_getpage(struct inode *inode, unsigned long idx,
> + struct page **pagep, enum sgp_type sgp, int *type);
> +
> +extern struct vm_operations_struct shmem_vm_ops;
> +extern struct vm_operations_struct shm_vm_ops;
...
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -103,8 +103,8 @@ static unsigned long shmem_default_max_inodes(void)
> }
> #endif
>
> -static int shmem_getpage(struct inode *inode, unsigned long idx,
> - struct page **pagep, enum sgp_type sgp, int *type);
> +int shmem_getpage(struct inode *inode, unsigned long idx,
> + struct page **pagep, enum sgp_type sgp, int *type);
Personally, I think it is bad form to export someone else's function for
them. Please move the current shmem.c function declaration to a header
that can be included by both the c/r code and shmem.c
> /* return the subtype of a shared vma segment */
> static enum vm_type cr_shared_vma_type(struct vm_area_struct *vma, int new)
> {
> enum vm_type vma_type;
>
> if (vma->vm_ops == &shmem_vm_ops) /* /dev/zero or anonymous */
> vma_type = CR_VMA_SHM_ANON;
> else if (vma->vm_ops == &shm_vm_ops) /* IPC (unsupported yet) */
> vma_type = -EINVAL;
> else
> vma_type = CR_VMA_SHM_FILE;
>
> if (!new && vma_type == CR_VMA_SHM_ANON)
> vma_type = CR_VMA_SHM_ANON_SKIP;
>
> return vma_type;
> }
This just looks really fragile to me. The first thing that comes up in
my cscope is bf5xx_pcm_mmap(). That is VM_SHARED, but is *certainly*
not CR_VMA_SHM_FILE.
Instead of *checking* vma->vm_ops, I believe the more correct solutions
here is to add an new vm_op which can be asked for its CR_VMA_* type.
-- Dave
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
